Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Haohao/fix halt problem #40

Closed
wants to merge 12 commits into from

Conversation

vava24680
Copy link
Contributor

  • 修復當 exception 拉起來時整個程式 halt 的問題
  • 移除 Process class 中不必要覆寫的 methods,同時移除 Manager 並直接使用 event
  • 使用 signal 重寫 termination procedure

@vava24680 vava24680 requested review from iblislin and Ksoy December 7, 2020 02:27
@codecov
Copy link

codecov bot commented Dec 7, 2020

Codecov Report

Merging #40 (7b59cc2) into master (73e36c7) will increase coverage by 0.22%.
The diff coverage is 21.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #40      +/-   ##
==========================================
+ Coverage   38.47%   38.70%   +0.22%     
==========================================
  Files           6        6              
  Lines         512      509       -3     
  Branches       72       76       +4     
==========================================
  Hits          197      197              
+ Misses        308      306       -2     
+ Partials        7        6       -1     
Impacted Files Coverage Δ
iottalkpy/dai.py 38.46% <21.21%> (+0.58%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 73e36c7...7b59cc2. Read the comment docs.

@vava24680 vava24680 linked an issue Dec 7, 2020 that may be closed by this pull request
iottalkpy/dai.py Outdated
import sys
import time
import traceback

from multiprocessing import Process, Manager
from threading import Thread, Event
from multiprocessing import Event as multiprocessingEvent
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python Class 的話就 CacmelCase

iottalkpy/dai.py Outdated
@@ -25,6 +26,8 @@
except ImportError:
pass

terminate_event = multiprocessingEvent()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

這樣的話,是 global 變數,這個 class DAI 如果有多個 instance 會發生啥事情?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

應該不會發生什麼奇怪的事情吧,當 terminate_event 被拉起來的時候,所有 DAI instances 應該都會終止

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm 我在想這個 signal handler 會不會影響到 AG 的使用, AG 用 Django 跟 application server 打算用 UWSGI

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

為什麼會影響哇?

iottalkpy/dai.py Outdated
until it actually exits.

Ref:
1. https://stackoverflow.com/questions/43092371/ignore-sigint-in-python-multiprocessing-subprocess # noqa: E501
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

用這篇的哪個 answer? 直接給 answer link 吧

ret = super(DAI, self).start(*args, **kwargs)
# conduct deregistration properly,
# if one doesn't stop process before main process ends
atexit.register(self.terminate)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

這個被拔掉後,main thread 結束,沒有送出 deregister message

In [5]: m = dai.load_module('./sa.py')

In [6]: m
Out[6]: <module 'sa' from '/home/iblis/git/iottalk-py/tmp/sa.py'>

In [7]: da = dai.module_to_sa(m)

In [8]: da
Out[8]: <DAI(DAI-1, initial daemon)>

In [9]: da.start
Out[9]: <bound method BaseProcess.start of <DAI(DAI-1, initial daemon)>>
                                                                                                                                                                             
In [10]: da.start()
                                                                                                                                                                             
In [11]: INFO:DAN:Successfully connect to http://panettone.iottalk.tw:11150/csm.
INFO:DAN:Device ID: ff73f56c-4439-46e4-81eb-03372e08e18d.
INFO:DAN:Device name: 00.Dummy_Device.
register successfully
INFO:DAI:Press Ctrl+C to exit DAI.
In [11]: 
                                                                                                                                                                             
In [11]: 
                                                                                                                                                                             
In [11]:                                                                                                                                                                      
Do you really want to exit ([y]/n)? y
                                                                                                                                                                             
                                                                                                                                                                             
                                                                                                                                                                             
                                                                                                                                                                             

^CError in atexit._run_exitfuncs:
Traceback (most recent call last):
  File "/usr/lib/python3.6/multiprocessing/popen_fork.py", line 28, in poll
    pid, sts = os.waitpid(self.pid, flag)
KeyboardInterrupt

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我先把 PR 改成 Draft,之後來研究有沒有好的做法

@vava24680 vava24680 marked this pull request as draft December 10, 2020 09:46
@vava24680 vava24680 closed this Dec 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Subprocess 丟出 exception 導致程式整個 halt 住
2 participants