Skip to content

Commit

Permalink
fix: avoid deadlocks in Python forked workers
Browse files Browse the repository at this point in the history
  • Loading branch information
majorgreys committed Dec 23, 2021
1 parent eafb025 commit b6815df
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 64 deletions.
29 changes: 19 additions & 10 deletions core/master_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -747,9 +747,22 @@ int uwsgi_respawn_worker(int wid) {
pthread_mutex_lock(&uwsgi.threaded_logger_lock);
}


for (i = 0; i < 256; i++) {
if (uwsgi.p[i]->pre_uwsgi_fork) {
uwsgi.p[i]->pre_uwsgi_fork();
}
}

pid_t pid = uwsgi_fork(uwsgi.workers[wid].name);

if (pid == 0) {
for (i = 0; i < 256; i++) {
if (uwsgi.p[i]->post_uwsgi_fork) {
uwsgi.p[i]->post_uwsgi_fork(1);
}
}

signal(SIGWINCH, worker_wakeup);
signal(SIGTSTP, worker_wakeup);
uwsgi.mywid = wid;
Expand Down Expand Up @@ -792,16 +805,6 @@ int uwsgi_respawn_worker(int wid) {

uwsgi.my_signal_socket = uwsgi.workers[wid].signal_pipe[1];

if (uwsgi.master_process) {
if ((uwsgi.workers[uwsgi.mywid].respawn_count || uwsgi.status.is_cheap)) {
for (i = 0; i < 256; i++) {
if (uwsgi.p[i]->master_fixup) {
uwsgi.p[i]->master_fixup(1);
}
}
}
}

if (uwsgi.threaded_logger) {
pthread_mutex_unlock(&uwsgi.threaded_logger_lock);
}
Expand All @@ -812,6 +815,12 @@ int uwsgi_respawn_worker(int wid) {
uwsgi_error("fork()");
}
else {
for (i = 0; i < 256; i++) {
if (uwsgi.p[i]->post_uwsgi_fork) {
uwsgi.p[i]->post_uwsgi_fork(0);
}
}

// the pid is set only in the master, as the worker should never use it
uwsgi.workers[wid].pid = pid;

Expand Down
6 changes: 0 additions & 6 deletions core/mule.c
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,6 @@ void uwsgi_mule(int id) {

uwsgi_close_all_sockets();

for (i = 0; i < 256; i++) {
if (uwsgi.p[i]->master_fixup) {
uwsgi.p[i]->master_fixup(1);
}
}

for (i = 0; i < 256; i++) {
if (uwsgi.p[i]->post_fork) {
uwsgi.p[i]->post_fork();
Expand Down
8 changes: 0 additions & 8 deletions core/uwsgi.c
Original file line number Diff line number Diff line change
Expand Up @@ -3338,14 +3338,6 @@ int uwsgi_start(void *v_argv) {
}
}

// master fixup
for (i = 0; i < 256; i++) {
if (uwsgi.p[i]->master_fixup) {
uwsgi.p[i]->master_fixup(0);
}
}



struct uwsgi_spooler *uspool = uwsgi.spoolers;
while (uspool) {
Expand Down
80 changes: 40 additions & 40 deletions plugins/python/python_plugin.c
Original file line number Diff line number Diff line change
Expand Up @@ -428,17 +428,14 @@ void uwsgi_python_atexit() {

void uwsgi_python_post_fork() {

if (uwsgi.i_am_a_spooler) {
// Need to acquire the gil when no master process is used as first worker
// will not have been forked like others
if (!uwsgi.master_process && uwsgi.mywid == 1) {
UWSGI_GET_GIL
}
}

// reset python signal flags so child processes can trap signals
if (up.call_osafterfork) {
#ifdef HAS_NOT_PyOS_AfterFork_Child
PyOS_AfterFork();
#else
PyOS_AfterFork_Child();
#endif
if (uwsgi.i_am_a_spooler) {
UWSGI_GET_GIL
}

uwsgi_python_reset_random_seed();
Expand Down Expand Up @@ -1129,6 +1126,10 @@ void uwsgi_python_preinit_apps() {
up.loaders[LOADER_CALLABLE] = uwsgi_callable_loader;
up.loaders[LOADER_STRING_CALLABLE] = uwsgi_string_callable_loader;

// GIL was released in previous initialization steps but init_pyargv expects
// the GIL to be acquired
UWSGI_GET_GIL

init_pyargv();

init_uwsgi_embedded_module();
Expand Down Expand Up @@ -1174,14 +1175,13 @@ void uwsgi_python_preinit_apps() {
upli = upli->next;
}

// Release the GIL before moving on forward with initialization
UWSGI_RELEASE_GIL

}

void uwsgi_python_init_apps() {

// lazy ?
if (uwsgi.mywid > 0) {
UWSGI_GET_GIL;
}
UWSGI_GET_GIL;

// prepare for stack suspend/resume
if (uwsgi.async > 0) {
Expand Down Expand Up @@ -1290,32 +1290,35 @@ void uwsgi_python_init_apps() {
Py_INCREF(up.after_req_hook_args);
}
}
// lazy ?
if (uwsgi.mywid > 0) {
UWSGI_RELEASE_GIL;
}
UWSGI_RELEASE_GIL;

}

void uwsgi_python_master_fixup(int step) {

static int master_fixed = 0;
static int worker_fixed = 0;
void uwsgi_python_pre_uwsgi_fork() {
if (uwsgi.has_threads) {
// Acquire the gil and import lock before forking in order to avoid
// deadlocks in workers
UWSGI_GET_GIL
_PyImport_AcquireLock();
}
}

if (!uwsgi.master_process) return;

void uwsgi_python_post_uwsgi_fork(int step) {
if (uwsgi.has_threads) {
if (step == 0) {
if (!master_fixed) {
UWSGI_RELEASE_GIL;
master_fixed = 1;
}
}
// Release locks within master process
_PyImport_ReleaseLock();
UWSGI_RELEASE_GIL
}
else {
if (!worker_fixed) {
UWSGI_GET_GIL;
worker_fixed = 1;
}
// Ensure thread state and locks are cleaned up in child process
#ifdef HAS_NOT_PyOS_AfterFork_Child
PyOS_AfterFork();
#else
PyOS_AfterFork_Child();
#endif
}
}
}
Expand Down Expand Up @@ -1349,7 +1352,8 @@ void uwsgi_python_enable_threads() {
up.reset_ts = threaded_reset_ts;
}


// Release the newly created gil from call to PyEval_InitThreads above
UWSGI_RELEASE_GIL

uwsgi_log("python threads support enabled\n");

Expand Down Expand Up @@ -2043,13 +2047,6 @@ static int uwsgi_python_worker() {
if (!up.worker_override)
return 0;
UWSGI_GET_GIL;
// ensure signals can be used again from python
if (!up.call_osafterfork)
#ifdef HAS_NOT_PyOS_AfterFork_Child
PyOS_AfterFork();
#else
PyOS_AfterFork_Child();
#endif
FILE *pyfile = fopen(up.worker_override, "r");
if (!pyfile) {
uwsgi_error_open(up.worker_override);
Expand Down Expand Up @@ -2081,6 +2078,10 @@ struct uwsgi_plugin python_plugin = {
.alias = "python",
.modifier1 = 0,
.init = uwsgi_python_init,

.pre_uwsgi_fork = uwsgi_python_pre_uwsgi_fork,
.post_uwsgi_fork = uwsgi_python_post_uwsgi_fork,

.post_fork = uwsgi_python_post_fork,
.options = uwsgi_python_options,
.request = uwsgi_request_wsgi,
Expand All @@ -2090,7 +2091,6 @@ struct uwsgi_plugin python_plugin = {
.init_apps = uwsgi_python_init_apps,

.fixup = uwsgi_python_fixup,
.master_fixup = uwsgi_python_master_fixup,
.master_cycle = uwsgi_python_master_cycle,

.mount_app = uwsgi_python_mount_app,
Expand Down
1 change: 1 addition & 0 deletions plugins/python/uwsgi_python.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include <uwsgi.h>
#include <Python.h>
#include <pythread.h>

#include <frameobject.h>

Expand Down
2 changes: 2 additions & 0 deletions uwsgi.h
Original file line number Diff line number Diff line change
Expand Up @@ -1039,6 +1039,8 @@ struct uwsgi_plugin {
void *data;
void (*on_load) (void);
int (*init) (void);
void (*pre_uwsgi_fork) (void);
void (*post_uwsgi_fork) (int);
void (*post_init) (void);
void (*post_fork) (void);
struct uwsgi_option *options;
Expand Down

0 comments on commit b6815df

Please sign in to comment.