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

Maybe don't trust threading.current_thread() to tell us whether we're in the main thread for signal handling purposes #461

Closed
njsmith opened this Issue Feb 26, 2018 · 0 comments

Comments

Projects
None yet
1 participant
@njsmith
Member

njsmith commented Feb 26, 2018

As @SunDwarf discovered (chat log), in some extremely weird circumstances it's possible to be in the main thread, yet threading.current_thread() == threading.main_thread() returns False. This is problematic because the main thread is special with respect to signal handling, and we use this in several places to check which kind of signal behavior to expect.

Specifically the case that broke is: import logbook, which internally imports some internals of gevent, including gevent.threading. In gevent 1.2.2 (and probably earlier versions), this triggers some monkeypatching of threading internals. (You're not supposed to import this module directly; logbook does anyway.) In particular, it breaks threading.current_thread(), so it gives the wrong value.

This will be fixed in the next version of gevent, but that's not out yet. (See: gevent/gevent#984) Also it's possible that there are other cases where the threading and signal modules end up with different ideas of whether they're running in the main thread? It seems hard to imagine though. So it's not entirely clear whether this is worth fixing.

The fix would be to stop checking threading.current_thread() before calling signal functions, and instead when we want to know whether the signal module thinks we're in the main thread, ask it directly by calling some signal function. So either "EAFP" style where we just try: signal.signal(...) except ValueError: not_in_main_thread_fallback(), or we could keep the current code style by adding a trio._util.in_main_thread that does something like, trying to call signal.signal(signal.SIGINT, signal.getsignal(signal.SIGINT)) and checks whether it raises an exception or not.

Fuyukai added a commit to Fuyukai/trio that referenced this issue Feb 27, 2018

@njsmith njsmith closed this in #462 Mar 13, 2018

Fuyukai added a commit to Fuyukai/trio that referenced this issue May 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment