From c959e469342b15682cc0b6912de39b3d3802ab1d Mon Sep 17 00:00:00 2001 From: Wynn Wilkes Date: Tue, 16 May 2023 14:48:39 -0600 Subject: [PATCH 1/4] New python 3.11 specific implementation of the tracebacker - The existing tracebacker is periodically segfaulting on python 3.11. This implementation pushes more of the logic into python code, leveraging updates to the tracebacker module, so that the c code just has to iterate over list of strings returned back. Note that this does change the output format of the tracebacker to match the standard python format of the traceback.StackSummary.format() method, so anything parsing that output that will need to update it's logic to match. --- plugins/python/tracebacker.c | 137 +++++++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+) diff --git a/plugins/python/tracebacker.c b/plugins/python/tracebacker.c index 0679e5187..7b09b3def 100644 --- a/plugins/python/tracebacker.c +++ b/plugins/python/tracebacker.c @@ -55,6 +55,141 @@ char *uwsgi_python_get_thread_name(PyObject *thread_id) { return NULL; } +#ifdef UWSGI_PY311 + +void *uwsgi_python_tracebacker_thread(void *foobar) { + struct iovec iov[1]; + struct sockaddr_un so_sun; + socklen_t so_sun_len = 0; + PyObject *uwsgi_tracebacker_module = NULL, *uwsgi_tracebacker_code = NULL; + + // locks the GIL + PyObject *new_thread = uwsgi_python_setup_thread("uWSGITraceBacker"); + if (!new_thread) return NULL; + + char *str_wid = uwsgi_num2str(uwsgi.mywid); + char *sock_path = uwsgi_concat2(up.tracebacker, str_wid); + free(str_wid); + + int current_defer_accept = uwsgi.no_defer_accept; + uwsgi.no_defer_accept = 1; + int fd = bind_to_unix(sock_path, uwsgi.listen_queue, uwsgi.chmod_socket, uwsgi.abstract_socket); + uwsgi.no_defer_accept = current_defer_accept; + if (fd < 0) { + goto cleanup; + } + + char *uwsgi_tracebacker_codestr = + "import sys, threading, traceback\n" + "\n" + "def uwsgi_tracebacker_lines():\n" + " rval = []\n" + " frames = sys._current_frames()\n" + " for tid, f in frames.items():\n" + " threadname = threading._active.get(tid).name\n" + " if threadname.startswith('uWSGIWorker'):\n" + " ss = traceback.extract_stack(f)\n" + " rval.extend(ss.format())\n" + " return rval\n"; + uwsgi_tracebacker_code = Py_CompileString(uwsgi_tracebacker_codestr, "uwsgi_tracebacker", Py_file_input); + if (uwsgi_tracebacker_code) { + uwsgi_tracebacker_module = PyImport_ExecCodeModule("uwsgi_tracebacker", uwsgi_tracebacker_code); + if (!uwsgi_tracebacker_module) { + uwsgi_log("!!! Failed to create tracebacker module\n"); + PyErr_Print(); + } + } else { + if (PyErr_Occurred()) { + uwsgi_log("!!! failed to compile tracebacker string, error:\n"); + PyErr_Print(); + } else { + uwsgi_log("!!! failed to compile tracebacker string, unknown error\n"); + } + } + UWSGI_RELEASE_GIL; + + if (!uwsgi_tracebacker_code || !uwsgi_tracebacker_module) { + goto cleanup; + } + + uwsgi_log("** python3.11 tracebacker for worker %d available on %s\n", uwsgi.mywid, sock_path); + + while (uwsgi.shutdown_sockets == 0) { + int client_fd = accept(fd, (struct sockaddr *) &so_sun, &so_sun_len); + if (client_fd < 0) { + uwsgi_error("accept()"); + goto loopcontinue; + } + UWSGI_GET_GIL; + + PyObject *lines_iter = NULL, *line = NULL; + PyObject *lines = PyObject_CallMethod(uwsgi_tracebacker_module, "uwsgi_tracebacker_lines", NULL); + if (!lines) { + uwsgi_log("pytracebacker: uwsgi_tracebacker_lines"); + PyErr_Print(); + goto loopcleanup; + } + + lines_iter = PyObject_GetIter(lines); + if (!lines_iter) { + uwsgi_log("pytracebacker: lines iterator"); + PyErr_Print(); + goto loopcleanup; + } + + char *header = "*** uWSGI Python tracebacker output ***\n"; + int header_len = strlen(header); + line = PyIter_Next(lines_iter); + if (line != NULL) { + if (write(client_fd, header, header_len) < 0) { + uwsgi_error("write()"); + } + } + + while(line) { + PyObject *line_bytes = PyUnicode_AsEncodedString(line, "utf8", NULL); + char* line_bytes_cstr = PyBytes_AS_STRING(line_bytes); + if (line_bytes_cstr) { + iov[0].iov_base = line_bytes_cstr; + iov[0].iov_len = PyBytes_GET_SIZE(line_bytes); + if (writev(client_fd, iov, 1) < 0) { + uwsgi_error("writev()"); + } + } + + if (line_bytes) Py_DECREF(line_bytes); + Py_DECREF(line); + line = PyIter_Next(lines_iter); + } + + loopcleanup: + if (lines_iter) Py_DECREF(lines_iter); + if (lines) Py_DECREF(lines); + + UWSGI_RELEASE_GIL; + + loopcontinue: + if (client_fd >= 0) { + close(client_fd); + } + } + +cleanup: + if (uwsgi_tracebacker_module != NULL || uwsgi_tracebacker_code != NULL) { + UWSGI_GET_GIL; + if (uwsgi_tracebacker_module) Py_DECREF(uwsgi_tracebacker_module); + if (uwsgi_tracebacker_code) Py_DECREF(uwsgi_tracebacker_code); + UWSGI_RELEASE_GIL; + } + if (sock_path) free(sock_path); + if (fd >= 0) close(fd); + + return NULL; +} + +#else + +// old pre 3.11 version void *uwsgi_python_tracebacker_thread(void *foobar) { struct iovec iov[11]; @@ -288,3 +423,5 @@ void *uwsgi_python_tracebacker_thread(void *foobar) { } return NULL; } + +#endif \ No newline at end of file From f5aeafe423b1800ca6bc023d7798a6f31b9cef13 Mon Sep 17 00:00:00 2001 From: Wynn Wilkes Date: Thu, 14 Mar 2024 08:47:03 -0600 Subject: [PATCH 2/4] Allow the valgrind generator script to run with a different python version - You can now pass a different path for a different python installation. --- valgrind/valgrind-generate-sups.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/valgrind/valgrind-generate-sups.sh b/valgrind/valgrind-generate-sups.sh index 37bfe3f88..3e28cb37b 100755 --- a/valgrind/valgrind-generate-sups.sh +++ b/valgrind/valgrind-generate-sups.sh @@ -1,5 +1,7 @@ #!/bin/bash +pydir=${1:-/usr} + gensup() { for SUP in Cond Free Leak Overlap Addr1 Addr2 Addr4 Addr8 Addr16 Value1 Value2 Value4 Value8 Value16 ; do @@ -16,10 +18,10 @@ gensup() { while read SO ; do gensup libpython "$SO" -done < <(find /usr/lib*/ -type f -name libpython*) +done < <(find ${pydir}/lib*/ -type f -name libpython*) while read SO ; do gensup python "$SO" -done < <(find /usr/lib*/python*/ -type f -name \*.so) +done < <(find ${pydir}/lib*/python*/ -type f -name \*.so) From 827a47cdf3b90265fdfa44f6a20ac0adc59603e4 Mon Sep 17 00:00:00 2001 From: Wynn Wilkes Date: Thu, 14 Mar 2024 08:48:19 -0600 Subject: [PATCH 3/4] Fix a potential error with not releasing the gil in uwsgi_python_rpc - If this call fails, we need to release the gil before returning as we've obtained it. --- plugins/python/python_plugin.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugins/python/python_plugin.c b/plugins/python/python_plugin.c index c69abc18b..5dee04135 100644 --- a/plugins/python/python_plugin.c +++ b/plugins/python/python_plugin.c @@ -1727,8 +1727,10 @@ uint64_t uwsgi_python_rpc(void *func, uint8_t argc, char **argv, uint16_t argvs[ PyObject *pyargs = PyTuple_New(argc); PyObject *ret; - if (!pyargs) + if (!pyargs) { + UWSGI_RELEASE_GIL; return 0; + } for (i = 0; i < argc; i++) { PyTuple_SetItem(pyargs, i, PyString_FromStringAndSize(argv[i], argvs[i])); From 0eaaf7a10e1e17d6b87cfeba81174a6b8f64d6fc Mon Sep 17 00:00:00 2001 From: Wynn Wilkes Date: Fri, 15 Mar 2024 12:33:33 -0600 Subject: [PATCH 4/4] More work on the pytracebacker to deal with the python sys._current_frames() segfaults - This method is known to behave in strange ways when called from multithreaded code, and crashes a lot when doing harakiri tracebacks. This new implementation avoids calling sys._current_frames, and instead uses the PyThreadState that we already have for the main uwsgi worker thread. We get that, then pass that into our python function which will build the stack trace string for us. This avoids the segfaults and is much more stable than before. It only gets the traceback for the main thread which is the uwsgi worker thread, which is all we wanted anyway. --- plugins/python/tracebacker.c | 148 +++++++++++++++++------------------ 1 file changed, 70 insertions(+), 78 deletions(-) diff --git a/plugins/python/tracebacker.c b/plugins/python/tracebacker.c index 7b09b3def..6654ec9e8 100644 --- a/plugins/python/tracebacker.c +++ b/plugins/python/tracebacker.c @@ -58,14 +58,19 @@ char *uwsgi_python_get_thread_name(PyObject *thread_id) { #ifdef UWSGI_PY311 void *uwsgi_python_tracebacker_thread(void *foobar) { - struct iovec iov[1]; + struct iovec iov; struct sockaddr_un so_sun; socklen_t so_sun_len = 0; - PyObject *uwsgi_tracebacker_module = NULL, *uwsgi_tracebacker_code = NULL; + PyObject *new_thread = NULL, *globalsDict = NULL, *localsDict = NULL, *codeFunc = NULL; + + memset(&iov, 0, sizeof(iov)); - // locks the GIL - PyObject *new_thread = uwsgi_python_setup_thread("uWSGITraceBacker"); - if (!new_thread) return NULL; + // this function locks the GIL + new_thread = uwsgi_python_setup_thread("uWSGITraceBacker"); + if (!new_thread) { + UWSGI_RELEASE_GIL; + return NULL; + } char *str_wid = uwsgi_num2str(uwsgi.mywid); char *sock_path = uwsgi_concat2(up.tracebacker, str_wid); @@ -78,41 +83,38 @@ void *uwsgi_python_tracebacker_thread(void *foobar) { if (fd < 0) { goto cleanup; } - - char *uwsgi_tracebacker_codestr = - "import sys, threading, traceback\n" + + char *codestr = + "def uwsgi_tracebacker_process_frame(frame):\n" + " import io, traceback\n" "\n" - "def uwsgi_tracebacker_lines():\n" - " rval = []\n" - " frames = sys._current_frames()\n" - " for tid, f in frames.items():\n" - " threadname = threading._active.get(tid).name\n" - " if threadname.startswith('uWSGIWorker'):\n" - " ss = traceback.extract_stack(f)\n" - " rval.extend(ss.format())\n" - " return rval\n"; - uwsgi_tracebacker_code = Py_CompileString(uwsgi_tracebacker_codestr, "uwsgi_tracebacker", Py_file_input); - if (uwsgi_tracebacker_code) { - uwsgi_tracebacker_module = PyImport_ExecCodeModule("uwsgi_tracebacker", uwsgi_tracebacker_code); - if (!uwsgi_tracebacker_module) { - uwsgi_log("!!! Failed to create tracebacker module\n"); - PyErr_Print(); - } + " lines = 0\n" + " rval = io.StringIO()\n" + " rval.write('*** uWSGI Python tracebacker output ***\\n')\n" + " ss = traceback.extract_stack(frame)\n" + " sslines = ss.format()\n" + " if sslines:\n" + " rval.writelines(sslines)\n" + " lines += 1\n" + " return rval.getvalue() if lines > 0 else None\n"; + globalsDict = PyDict_New(); + localsDict = PyDict_New(); + PyObject *result = PyRun_String(codestr, Py_file_input, globalsDict, localsDict); + if (!result) { + uwsgi_log("!!! Failed to define uwsgi_tracebacker_lines function\n"); + PyErr_Print(); } else { - if (PyErr_Occurred()) { - uwsgi_log("!!! failed to compile tracebacker string, error:\n"); - PyErr_Print(); - } else { - uwsgi_log("!!! failed to compile tracebacker string, unknown error\n"); - } + codeFunc = PyDict_GetItemString(localsDict, "uwsgi_tracebacker_process_frame"); + Py_DECREF(result); } UWSGI_RELEASE_GIL; - if (!uwsgi_tracebacker_code || !uwsgi_tracebacker_module) { + if (!codeFunc) { + uwsgi_log("** failed to initialize python tracebacker thread for worker %d'n", uwsgi.mywid); goto cleanup; } - uwsgi_log("** python3.11 tracebacker for worker %d available on %s\n", uwsgi.mywid, sock_path); + uwsgi_log("** python3.11 tracebacker for worker %d listening on %s\n", uwsgi.mywid, sock_path); while (uwsgi.shutdown_sockets == 0) { int client_fd = accept(fd, (struct sockaddr *) &so_sun, &so_sun_len); @@ -120,52 +122,41 @@ void *uwsgi_python_tracebacker_thread(void *foobar) { uwsgi_error("accept()"); goto loopcontinue; } - UWSGI_GET_GIL; - PyObject *lines_iter = NULL, *line = NULL; - PyObject *lines = PyObject_CallMethod(uwsgi_tracebacker_module, "uwsgi_tracebacker_lines", NULL); - if (!lines) { - uwsgi_log("pytracebacker: uwsgi_tracebacker_lines"); - PyErr_Print(); - goto loopcleanup; - } - - lines_iter = PyObject_GetIter(lines); - if (!lines_iter) { - uwsgi_log("pytracebacker: lines iterator"); - PyErr_Print(); - goto loopcleanup; - } + UWSGI_GET_GIL; - char *header = "*** uWSGI Python tracebacker output ***\n"; - int header_len = strlen(header); - line = PyIter_Next(lines_iter); - if (line != NULL) { - if (write(client_fd, header, header_len) < 0) { - uwsgi_error("write()"); - } - } - - while(line) { - PyObject *line_bytes = PyUnicode_AsEncodedString(line, "utf8", NULL); - char* line_bytes_cstr = PyBytes_AS_STRING(line_bytes); - if (line_bytes_cstr) { - iov[0].iov_base = line_bytes_cstr; - iov[0].iov_len = PyBytes_GET_SIZE(line_bytes); - if (writev(client_fd, iov, 1) < 0) { - uwsgi_error("writev()"); + PyFrameObject *threadFrame = PyThreadState_GetFrame(up.main_thread); + if (threadFrame) { + PyObject *tb = PyObject_CallFunctionObjArgs(codeFunc, threadFrame, NULL); + if (!tb) { + uwsgi_log("pytracebacker(%d): uwsgi_tracebacker failed\n", uwsgi.mywid); + PyErr_Print(); + } else { + if (tb != Py_None) { + PyObject *tb_bytes = PyUnicode_AsUTF8String(tb); + if (tb_bytes) { + char* tb_bytes_cstr = PyBytes_AS_STRING(tb_bytes); // internal ptr, don't free + int tb_bytes_len = PyBytes_GET_SIZE(tb_bytes); + if (tb_bytes_cstr) { + iov.iov_base = tb_bytes_cstr; + iov.iov_len = tb_bytes_len; + int bytes_written = writev(client_fd, &iov, 1); + if (bytes_written < 0) { + uwsgi_error("writev()"); + } else if (bytes_written < tb_bytes_len) { + uwsgi_log("pytracebacker(%d): only wrote %d/%d bytes\n", uwsgi.mywid, bytes_written, tb_bytes_len); + } else { + uwsgi_log("**!! pytracebacker(%d): sent traceback\n", uwsgi.mywid); + } + } + Py_DECREF(tb_bytes); + } } + Py_DECREF(tb); } - - if (line_bytes) Py_DECREF(line_bytes); - Py_DECREF(line); - line = PyIter_Next(lines_iter); + Py_DECREF(threadFrame); } - loopcleanup: - if (lines_iter) Py_DECREF(lines_iter); - if (lines) Py_DECREF(lines); - UWSGI_RELEASE_GIL; loopcontinue: @@ -175,12 +166,13 @@ void *uwsgi_python_tracebacker_thread(void *foobar) { } cleanup: - if (uwsgi_tracebacker_module != NULL || uwsgi_tracebacker_code != NULL) { - UWSGI_GET_GIL; - if (uwsgi_tracebacker_module) Py_DECREF(uwsgi_tracebacker_module); - if (uwsgi_tracebacker_code) Py_DECREF(uwsgi_tracebacker_code); - UWSGI_RELEASE_GIL; - } + UWSGI_GET_GIL; + Py_XDECREF(codeFunc); + Py_XDECREF(globalsDict); + Py_XDECREF(localsDict); + Py_DECREF(new_thread); + UWSGI_RELEASE_GIL; + if (sock_path) free(sock_path); if (fd >= 0) close(fd);