Skip to content

Commit

Permalink
fix(python-plugin): add uwsgi fork hooks to update internal interpret…
Browse files Browse the repository at this point in the history
…er state
  • Loading branch information
majorgreys committed Dec 29, 2021
1 parent eafb025 commit 3b4dcdd
Show file tree
Hide file tree
Showing 11 changed files with 106 additions and 5 deletions.
19 changes: 19 additions & 0 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 @@ -812,6 +825,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
79 changes: 74 additions & 5 deletions plugins/python/python_plugin.c
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ struct uwsgi_option uwsgi_python_options[] = {
#endif

{"py-call-osafterfork", no_argument, 0, "enable child processes running cpython to trap OS signals", uwsgi_opt_true, &up.call_osafterfork, 0},
{"py-call-uwsgi-fork-hooks", no_argument, 0, "call pre and post hooks when uswgi forks to update the internal interpreter state of CPython", uwsgi_opt_true, &up.call_uwsgi_fork_hooks, 0},

{"early-python", no_argument, 0, "load the python VM as soon as possible (useful for the fork server)", uwsgi_early_python, NULL, UWSGI_OPT_IMMEDIATE},
{"early-pyimport", required_argument, 0, "import a python module in the early phase", uwsgi_early_python_import, NULL, UWSGI_OPT_IMMEDIATE},
Expand Down Expand Up @@ -428,12 +429,20 @@ void uwsgi_python_atexit() {

void uwsgi_python_post_fork() {

// Need to acquire the gil when no master process is used as first worker
// will not have been forked like others
// Necessary if uwsgi fork hooks called to update interpreter state
if (up.call_uwsgi_fork_hooks && !uwsgi.master_process && uwsgi.mywid == 1) {
UWSGI_GET_GIL
}

if (uwsgi.i_am_a_spooler) {
UWSGI_GET_GIL
}

// reset python signal flags so child processes can trap signals
if (up.call_osafterfork) {
// Necessary if uwsgi fork hooks not called to update interpreter state
if (!up.call_uwsgi_fork_hooks && up.call_osafterfork) {
#ifdef HAS_NOT_PyOS_AfterFork_Child
PyOS_AfterFork();
#else
Expand Down Expand Up @@ -1129,6 +1138,13 @@ 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
// Necessary if uwsgi fork hooks called to update interpreter state
if (up.call_uwsgi_fork_hooks) {
UWSGI_GET_GIL
}

init_pyargv();

init_uwsgi_embedded_module();
Expand Down Expand Up @@ -1174,12 +1190,19 @@ void uwsgi_python_preinit_apps() {
upli = upli->next;
}

// Release the GIL before moving on forward with initialization
// Necessary if uwsgi fork hooks called to update interpreter state
if (up.call_uwsgi_fork_hooks) {
UWSGI_RELEASE_GIL
}

}

void uwsgi_python_init_apps() {

// lazy ?
if (uwsgi.mywid > 0) {
// Also necessary if uwsgi fork hooks called to update interpreter state
if (uwsgi.mywid > 0 || up.call_uwsgi_fork_hooks) {
UWSGI_GET_GIL;
}

Expand Down Expand Up @@ -1291,7 +1314,8 @@ void uwsgi_python_init_apps() {
}
}
// lazy ?
if (uwsgi.mywid > 0) {
// Also necessary if uwsgi fork hooks called to update interpreter state
if (uwsgi.mywid > 0 || up.call_uwsgi_fork_hooks) {
UWSGI_RELEASE_GIL;
}

Expand All @@ -1304,6 +1328,10 @@ void uwsgi_python_master_fixup(int step) {

if (!uwsgi.master_process) return;

// Skip master fixup if uwsgi fork hooks called to update interpreter state
if (up.call_uwsgi_fork_hooks)
return;

if (uwsgi.has_threads) {
if (step == 0) {
if (!master_fixed) {
Expand All @@ -1320,6 +1348,40 @@ void uwsgi_python_master_fixup(int step) {
}
}

void uwsgi_python_pre_uwsgi_fork() {
if (!up.call_uwsgi_fork_hooks)
return;

if (uwsgi.has_threads) {
// Acquire the gil and import lock before forking in order to avoid
// deadlocks in workers
UWSGI_GET_GIL
_PyImport_AcquireLock();
}
}


void uwsgi_python_post_uwsgi_fork(int step) {
if (!up.call_uwsgi_fork_hooks)
return;

if (uwsgi.has_threads) {
if (step == 0) {
// Release locks within master process
_PyImport_ReleaseLock();
UWSGI_RELEASE_GIL
}
else {
// Ensure thread state and locks are cleaned up in child process
#ifdef HAS_NOT_PyOS_AfterFork_Child
PyOS_AfterFork();
#else
PyOS_AfterFork_Child();
#endif
}
}
}

void uwsgi_python_enable_threads() {

#ifdef UWSGI_SHOULD_CALL_PYEVAL_INITTHREADS
Expand Down Expand Up @@ -1349,7 +1411,11 @@ void uwsgi_python_enable_threads() {
up.reset_ts = threaded_reset_ts;
}


// Release the newly created gil from call to PyEval_InitThreads above
// Necessary if uwsgi fork hooks called to update interpreter state
if (up.call_uwsgi_fork_hooks) {
UWSGI_RELEASE_GIL
}

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

Expand Down Expand Up @@ -2044,7 +2110,8 @@ static int uwsgi_python_worker() {
return 0;
UWSGI_GET_GIL;
// ensure signals can be used again from python
if (!up.call_osafterfork)
// Necessary if fork hooks have been not used to update interpreter state
if (!up.call_osafterfork && !up.call_uwsgi_fork_hooks)
#ifdef HAS_NOT_PyOS_AfterFork_Child
PyOS_AfterFork();
#else
Expand Down Expand Up @@ -2130,4 +2197,6 @@ struct uwsgi_plugin python_plugin = {

.worker = uwsgi_python_worker,

.pre_uwsgi_fork = uwsgi_python_pre_uwsgi_fork,
.post_uwsgi_fork = uwsgi_python_post_uwsgi_fork,
};
3 changes: 3 additions & 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 Expand Up @@ -238,6 +239,8 @@ struct uwsgi_python {
int master_check_signals;

char *executable;

int call_uwsgi_fork_hooks;
};


Expand Down
1 change: 1 addition & 0 deletions tests/deadlocks/master-nothreads.ini
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ show-config = true
master = true
enable-threads = false
workers = 1
py-call-uwsgi-fork-hooks = true
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ master = true
enable-threads = true
workers = 10
single-interpreter = true
py-call-uwsgi-fork-hooks = true
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ master = true
enable-threads = true
workers = 1
single-interpreter = true
py-call-uwsgi-fork-hooks = true
1 change: 1 addition & 0 deletions tests/deadlocks/master-threads-10workers.ini
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ show-config = true
master = true
enable-threads = true
workers = 10
py-call-uwsgi-fork-hooks = true
1 change: 1 addition & 0 deletions tests/deadlocks/master-threads-1worker.ini
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ show-config = true
master = true
enable-threads = true
workers = 1
py-call-uwsgi-fork-hooks = true
1 change: 1 addition & 0 deletions tests/deadlocks/nomaster-threads-10workers.ini
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ show-config = true
master = false
enable-threads = true
workers = 10
py-call-uwsgi-fork-hooks = true
1 change: 1 addition & 0 deletions tests/deadlocks/nomaster-threads-1worker.ini
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ show-config = true
master = false
enable-threads = true
workers = 1
py-call-uwsgi-fork-hooks = true
3 changes: 3 additions & 0 deletions uwsgi.h
Original file line number Diff line number Diff line change
Expand Up @@ -1095,6 +1095,9 @@ struct uwsgi_plugin {
int (*worker)(void);

void (*early_post_jail) (void);

void (*pre_uwsgi_fork) (void);
void (*post_uwsgi_fork) (int);
};

#ifdef UWSGI_PCRE
Expand Down

0 comments on commit 3b4dcdd

Please sign in to comment.