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

Adding optional SIGINT handler. It raises RuntimeError when run on a … #9

Merged
merged 1 commit into from
Aug 20, 2020

Conversation

quancore
Copy link

@quancore quancore commented Jul 2, 2020

Adding an optional SIGINT handler. Default SIGINT handler addition is problematical on a thread other then main thread since add_signal_handler only work in the main thread.

…thread other then main thread since add_signal_handler only work in the main thread
@sonarcloud
Copy link

sonarcloud bot commented Jul 2, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@bluet bluet added the enhancement New feature or request label Jul 12, 2020
@afunTW
Copy link
Collaborator

afunTW commented Jul 18, 2020

Reference to the add_signal_handler issue https://bugs.python.org/issue34679

@afunTW
Copy link
Collaborator

afunTW commented Jul 18, 2020

According to the comment in the Python issue thread, it seems only happened in python 3.8.

The issue is related to Python 3.8 and master only. 3.6-3.7 are not affected - Andrew Svetlov

@bluet Is this project going to support python 3.8 in the future?
@quancore could you please provide more information including the environment? how do we reproduce the error?

@quancore
Copy link
Author

My env was Ubuntu, python 3.7. So the problem also persist for python 3.7. To reproduce runs the proxy broker on a thread.
Also please check: constverum#109

@afunTW afunTW requested review from afunTW and bluet August 7, 2020 01:44
@bluet
Copy link
Owner

bluet commented Aug 15, 2020

@afunTW 你幫我決定一下 XD
如果覺得 ok ,這個 merge 並不會產生危害(且能解決使用者的問題)的話,可以直接 merge

@afunTW
Copy link
Collaborator

afunTW commented Aug 20, 2020

@bluet Since the same issue and PR happened in source repo constverum#109
It's better to handle this issue so far although we still not able to interrupt in non-main thread.

LGTM

@afunTW afunTW merged commit a6e3fd9 into bluet:master Aug 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants