Skip to content

bpo-40010: Optimize signal handling in multithreaded applications #19067

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

Merged
merged 1 commit into from
Mar 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions Doc/whatsnew/3.9.rst
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,16 @@ Optimizations

(Contributed by Serhiy Storchaka in :issue:`32856`.)

* Optimize signal handling in multithreaded applications. If a thread different
than the main thread gets a signal, the bytecode evaluation loop is no longer
interrupted at each bytecode instruction to check for pending signals which
cannot be handled. Only the main thread of the main interpreter can handle
signals.

Previously, the bytecode evaluation loop was interrupted at each instruction
until the main thread handles signals.
(Contributed by Victor Stinner in :issue:`40010`.)


Build and C API Changes
=======================
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Optimize signal handling in multithreaded applications. If a thread different
than the main thread gets a signal, the bytecode evaluation loop is no longer
interrupted at each bytecode instruction to check for pending signals which
cannot be handled. Only the main thread of the main interpreter can handle
signals.

Previously, the bytecode evaluation loop was interrupted at each instruction
until the main thread handles signals.
18 changes: 14 additions & 4 deletions Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,24 @@ static size_t opcache_global_hits = 0;
static size_t opcache_global_misses = 0;
#endif


/* Only handle signals on the main thread of the main interpreter. */
static int
thread_can_handle_signals(void)
Copy link
Contributor

@aeros aeros Mar 19, 2020

Choose a reason for hiding this comment

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

The following seems a bit more specifically descriptive of what it's doing, and would make more sense if it's ever used in the future for something other than signal handling:

Suggested change
thread_can_handle_signals(void)
current_is_main_thread(void)

Copy link
Member Author

Choose a reason for hiding this comment

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

Even if only the main thread of the main interpreter can handle signals, I prefer "thread_can_handle_signals" name to make the function intent more explicit.

{
return (PyThread_get_thread_ident() == _PyRuntime.main_thread);
}


/* This can set eval_breaker to 0 even though gil_drop_request became
1. We believe this is all right because the eval loop will release
the GIL eventually anyway. */
#define COMPUTE_EVAL_BREAKER(ceval, ceval2) \
_Py_atomic_store_relaxed( \
&(ceval2)->eval_breaker, \
_Py_atomic_load_relaxed(&(ceval)->gil_drop_request) | \
_Py_atomic_load_relaxed(&(ceval)->signals_pending) | \
(_Py_atomic_load_relaxed(&(ceval)->signals_pending) \
&& thread_can_handle_signals()) | \
_Py_atomic_load_relaxed(&(ceval2)->pending.calls_to_do) | \
(ceval2)->pending.async_exc)

Expand Down Expand Up @@ -156,10 +166,11 @@ static size_t opcache_global_misses = 0;
COMPUTE_EVAL_BREAKER(ceval, ceval2); \
} while (0)

/* eval_breaker is not set to 1 if thread_can_handle_signals() is false. */
#define SIGNAL_PENDING_SIGNALS(ceval, ceval2) \
do { \
_Py_atomic_store_relaxed(&(ceval)->signals_pending, 1); \
_Py_atomic_store_relaxed(&(ceval2)->eval_breaker, 1); \
COMPUTE_EVAL_BREAKER(ceval, ceval2); \
} while (0)

#define UNSIGNAL_PENDING_SIGNALS(ceval, ceval2) \
Expand Down Expand Up @@ -540,8 +551,7 @@ handle_signals(PyThreadState *tstate)
{
_PyRuntimeState *runtime = tstate->interp->runtime;

/* Only handle signals on main thread */
if (PyThread_get_thread_ident() != runtime->main_thread) {
if (!thread_can_handle_signals()) {
return 0;
}
/*
Expand Down
9 changes: 9 additions & 0 deletions Python/ceval_gil.h
Original file line number Diff line number Diff line change
Expand Up @@ -280,9 +280,18 @@ take_gil(PyThreadState *tstate)
COND_SIGNAL(gil->switch_cond);
MUTEX_UNLOCK(gil->switch_mutex);
#endif

if (_Py_atomic_load_relaxed(&ceval->gil_drop_request)) {
RESET_GIL_DROP_REQUEST(ceval, ceval2);
}
else {
/* bpo-40010: eval_breaker should be recomputed to be set to 1 if there
a pending signal: signal received by another thread which cannot
handle signals.

Note: RESET_GIL_DROP_REQUEST() calls COMPUTE_EVAL_BREAKER(). */
COMPUTE_EVAL_BREAKER(ceval, ceval2);
}

int must_exit = tstate_must_exit(tstate);

Expand Down