From e579e0288058a5650dcd46585581f164db430f71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D0=BB=D0=B5=D0=BA=D1=81=D0=B0=D0=BD=D0=B4=D1=80=20?= =?UTF-8?q?=D0=9C=D0=B5=D0=BD=D1=89=D0=B8=D0=BA=D0=BE=D0=B2?= Date: Wed, 8 Dec 2021 21:49:53 +0300 Subject: [PATCH 1/2] fix incorrect main thread id value in mp.Process --- uvloop/includes/stdlib.pxi | 2 +- uvloop/loop.pxd | 1 - uvloop/loop.pyx | 9 ++------- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/uvloop/includes/stdlib.pxi b/uvloop/includes/stdlib.pxi index adf9806b..3c27687f 100644 --- a/uvloop/includes/stdlib.pxi +++ b/uvloop/includes/stdlib.pxi @@ -135,8 +135,8 @@ cdef int ssl_SSL_ERROR_WANT_READ = ssl.SSL_ERROR_WANT_READ cdef int ssl_SSL_ERROR_WANT_WRITE = ssl.SSL_ERROR_WANT_WRITE cdef int ssl_SSL_ERROR_SYSCALL = ssl.SSL_ERROR_SYSCALL -cdef uint64_t MAIN_THREAD_ID = threading.main_thread().ident cdef threading_Thread = threading.Thread +cdef threading_main_thread = threading.main_thread cdef int subprocess_PIPE = subprocess.PIPE cdef int subprocess_STDOUT = subprocess.STDOUT diff --git a/uvloop/loop.pxd b/uvloop/loop.pxd index 06bee851..d3c91709 100644 --- a/uvloop/loop.pxd +++ b/uvloop/loop.pxd @@ -44,7 +44,6 @@ cdef class Loop: bint _stopping uint64_t _thread_id - bint _thread_is_main object _task_factory object _exception_handler diff --git a/uvloop/loop.pyx b/uvloop/loop.pyx index d9b5aaac..bde07a73 100644 --- a/uvloop/loop.pyx +++ b/uvloop/loop.pyx @@ -141,7 +141,6 @@ cdef class Loop: self._closed = 0 self._debug = 0 - self._thread_is_main = 0 self._thread_id = 0 self._running = 0 self._stopping = 0 @@ -215,7 +214,7 @@ cdef class Loop: self._servers = set() cdef inline _is_main_thread(self): - return MAIN_THREAD_ID == PyThread_get_thread_ident() + return threading_main_thread().ident == PyThread_get_thread_ident() def __init__(self): self.set_debug((not sys_ignore_environment @@ -519,7 +518,6 @@ cdef class Loop: self._last_error = None self._thread_id = PyThread_get_thread_ident() - self._thread_is_main = MAIN_THREAD_ID == self._thread_id self._running = 1 self.handler_check__exec_writes.start() @@ -540,7 +538,6 @@ cdef class Loop: self._pause_signals() - self._thread_is_main = 0 self._thread_id = 0 self._running = 0 self._stopping = 0 @@ -3281,9 +3278,6 @@ cdef Loop __forking_loop = None cdef void __get_fork_handler() nogil: - global __forking - global __forking_loop - with gil: if (__forking and __forking_loop is not None and __forking_loop.active_process_handler is not None): @@ -3291,6 +3285,7 @@ cdef void __get_fork_handler() nogil: cdef __install_atfork(): global __atfork_installed + if __atfork_installed: return __atfork_installed = 1 From 34cb675d9ecd299aa01f1c3e383b1e40f57ed577 Mon Sep 17 00:00:00 2001 From: Fantix King Date: Wed, 31 Aug 2022 12:31:06 -0700 Subject: [PATCH 2/2] Make MAIN_THREAD_ID a lazy value and add test --- tests/test_signals.py | 38 +++++++++++++++++++++++++++++++++- uvloop/includes/fork_handler.h | 11 ++++++++++ uvloop/includes/system.pxd | 5 +++++ uvloop/loop.pyx | 6 +++++- 4 files changed, 58 insertions(+), 2 deletions(-) diff --git a/tests/test_signals.py b/tests/test_signals.py index e51f5691..9ee972fe 100644 --- a/tests/test_signals.py +++ b/tests/test_signals.py @@ -310,7 +310,7 @@ async def coro(): with self.assertRaisesRegex(TypeError, 'coroutines cannot be used'): self.loop.add_signal_handler(signal.SIGHUP, coro) - def test_wakeup_fd_unchanged(self): + def test_signals_wakeup_fd_unchanged(self): async def runner(): PROG = R"""\ import uvloop @@ -349,6 +349,42 @@ async def f(): pass self.loop.run_until_complete(runner()) + def test_signals_fork_in_thread(self): + # Refs #452, when forked from a thread, the main-thread-only signal + # operations failed thread ID checks because we didn't update + # MAIN_THREAD_ID after fork. It's now a lazy value set when needed and + # cleared after fork. + PROG = R"""\ +import asyncio +import multiprocessing +import signal +import sys +import threading +import uvloop + +multiprocessing.set_start_method('fork') + +def subprocess(): + loop = """ + self.NEW_LOOP + """ + loop.add_signal_handler(signal.SIGINT, lambda *a: None) + +def run(): + loop = """ + self.NEW_LOOP + """ + loop.add_signal_handler(signal.SIGINT, lambda *a: None) + p = multiprocessing.Process(target=subprocess) + t = threading.Thread(target=p.start) + t.start() + t.join() + p.join() + sys.exit(p.exitcode) + +run() +""" + + subprocess.check_call([ + sys.executable, b'-W', b'ignore', b'-c', PROG, + ]) + class Test_UV_Signals(_TestSignal, tb.UVTestCase): NEW_LOOP = 'uvloop.new_event_loop()' diff --git a/uvloop/includes/fork_handler.h b/uvloop/includes/fork_handler.h index 19a016d5..47bbe036 100644 --- a/uvloop/includes/fork_handler.h +++ b/uvloop/includes/fork_handler.h @@ -1,3 +1,5 @@ +volatile uint64_t MAIN_THREAD_ID = 0; +volatile int8_t MAIN_THREAD_ID_SET = 0; typedef void (*OnForkHandler)(); @@ -9,6 +11,10 @@ Note: Fork handler needs to be in C (not cython) otherwise it would require GIL to be present, but some forks can exec non-python processes. */ void handleAtFork(void) { + // Reset the MAIN_THREAD_ID on fork, because the main thread ID is not + // always the same after fork, especially when forked from within a thread. + MAIN_THREAD_ID_SET = 0; + if (__forkHandler != NULL) { __forkHandler(); } @@ -25,3 +31,8 @@ void resetForkHandler(void) { __forkHandler = NULL; } + +void setMainThreadID(uint64_t id) { + MAIN_THREAD_ID = id; + MAIN_THREAD_ID_SET = 1; +} diff --git a/uvloop/includes/system.pxd b/uvloop/includes/system.pxd index 63b9efa0..367fedd1 100644 --- a/uvloop/includes/system.pxd +++ b/uvloop/includes/system.pxd @@ -1,3 +1,5 @@ +from libc.stdint cimport int8_t, uint64_t + cdef extern from "arpa/inet.h" nogil: int ntohl(int) @@ -85,7 +87,10 @@ cdef extern from "includes/compat.h" nogil: cdef extern from "includes/fork_handler.h": + uint64_t MAIN_THREAD_ID + int8_t MAIN_THREAD_ID_SET ctypedef void (*OnForkHandler)() void handleAtFork() void setForkHandler(OnForkHandler handler) void resetForkHandler() + void setMainThreadID(uint64_t id) diff --git a/uvloop/loop.pyx b/uvloop/loop.pyx index b2364ee1..5dff98a1 100644 --- a/uvloop/loop.pyx +++ b/uvloop/loop.pyx @@ -215,7 +215,11 @@ cdef class Loop: self._servers = set() cdef inline _is_main_thread(self): - return threading_main_thread().ident == PyThread_get_thread_ident() + cdef uint64_t main_thread_id = system.MAIN_THREAD_ID + if system.MAIN_THREAD_ID_SET == 0: + main_thread_id = threading_main_thread().ident + system.setMainThreadID(main_thread_id) + return main_thread_id == PyThread_get_thread_ident() def __init__(self): self.set_debug((not sys_ignore_environment