Fix fork-child hangs on copied condition variables (musl); skip re-testing identical trees on develop

.gitlab-ci.yml

@@ -23,6 +23,8 @@ default:
     - k8s
     - amd64
   before_script:
+    # a test job of a develop push whose tree already passed every test job reports success and ends here
+    - . test/docker_test/tree-tested-skip.sh
     - !reference [.github-status, script]
     # the job runs and reports its result even if the pending state cannot be set
     - github_status pending "${REPO_NAME}/${CI_JOB_NAME}" "${CI_JOB_URL}" || echo "cannot set the GitHub status" >&2
@@ -31,6 +33,27 @@ default:
 variables:
   REPO_NAME: module-python
 
+# Reuse successful tests only for a push to develop with an identical Git tree (as in module-ncurses 73d328c and
+# module-pgsql 251bac2): tree-already-tested.py finds a successful pipeline in which every test job passed for the
+# commit or for a parent with the same tree (a merged pull request branch), and each test job then sources
+# tree-tested-skip.sh, which reports the job's success to GitHub with a link to that pipeline and ends the job.  The
+# github-status-* jobs still set the pipeline's GitHub status.  Missing/incomplete API evidence runs every test job in
+# full; QORE_CI_FORCE_FULL=1 forces it.  The REQUIRED_JOBS of tree-already-tested.py must list every test job
+# (test/test_tree_tested.py checks it).
+tree-already-tested:
+  stage: .pre
+  image: $CI_REGISTRY/infrastructure/qore-test-base/qore-test-base:develop
+  before_script: []
+  variables:
+    GIT_DEPTH: "3"
+  rules:
+    - if: '$CI_COMMIT_BRANCH == "develop" && $CI_PIPELINE_SOURCE == "push" && $QORE_CI_FORCE_FULL != "1"'
+  script:
+    - python3 test/docker_test/tree-already-tested.py
+  artifacts:
+    reports:
+      dotenv: tree-tested.env
+
 .github-pipeline-status:
   image: $CI_REGISTRY/infrastructure/qore-test-base/qore-test-base:develop
   before_script:
@@ -60,10 +83,11 @@ github-status-failure:
 
 .test-ubuntu:
   stage: test
+  needs: [{job: tree-already-tested, optional: true}]
   image: $CI_REGISTRY/infrastructure/qore-test-base/qore-test-base:develop
   script:
     - |
-        if test/docker_test/test-ubuntu.sh; then
+        if python3 test/test_tree_tested.py && test/docker_test/test-ubuntu.sh; then
           github_status success "${REPO_NAME}/${CI_JOB_NAME}" "${CI_JOB_URL}"
           exit 0
         else
@@ -104,10 +128,11 @@ test-ubuntu-python3.16:
 
 test-alpine:
   stage: test
+  needs: [{job: tree-already-tested, optional: true}]
   image: $CI_REGISTRY/infrastructure/qore-test-base/qore-test-base:develop-alpine
   script:
     - |
-        if test/docker_test/test-alpine.sh; then
+        if python3 test/test_tree_tested.py && test/docker_test/test-alpine.sh; then
           github_status success "${REPO_NAME}/${CI_JOB_NAME}" "${CI_JOB_URL}"
           exit 0
         else

design/python-module.md

@@ -639,6 +639,7 @@ process-wide locks:
 |---|---|---|
 | `QorePythonProgram::py_thr_lck` | `QFR_MODULE_LEAF` (acquired) | holders only change containers and never wait for the GIL while holding it |
 | `qpy_lock`, `qpy_global_lock` | `QFR_MODULE_LEAF` (acquired) | holders only change the program sets |
+| `DeclHandleRegistry::lck` | `QFR_MODULE_LEAF` (acquired) | holders only change the handle registry and use counts; the child keeps only the forking thread's uses |
 | `QorePythonProgram::main_ts_lck` | `QFR_MODULE_RESET` | holders wait for the GIL (interpreter creation), which a thread in `os.fork()` holds while the handlers run; the child takes it again if the forking thread held it |
 
 The GIL interplay, measured on Linux with Python 3.14 (GIL build):
@@ -697,6 +698,20 @@ The GIL interplay, measured on Linux with Python 3.14 (GIL build):
   thread states other than the finalizing one when it is deleted in the child (states of the parent's Python
   threads) is left in memory instead of being ended: the threading module, which is not told of a fork() that Python
   does not make, would wait for those threads forever in `Py_EndInterpreter()`.
+- Every condition variable that a thread of the parent can wait on is constructed again in the child, in place and
+  without destroying the old object: the child's copy still lists the waiting threads, which do not exist in the
+  child, and musl's `pthread_cond_signal()` / `pthread_cond_broadcast()` (and `pthread_cond_destroy()`) wait for the
+  waiters that they wake up, so the child hung in its first signal (`QoreCondition::reinitAfterFork()` does the same
+  in libqore, but modules cannot call it).  This covers `GilSwitchState::cond` (`gil_switch_fork_child()`, a
+  `pthread_atfork()` child handler: the helper thread of `QorePythonGilWait` waits on it whenever the prepare handler
+  holds its lock, and is started again in the child), `pgm_thr_cond` of every interpreter (`forkChildThreadStates()`,
+  which also clears `pgm_thr_waiting`), and the condition of the declaration handles (see below).
+- The child handler of the lock of the declaration handles (`QorePythonDeclHandle::forkChild()`) clears the number
+  of threads waiting in `invalidate()`, and sets the number of uses of every handle to the uses of the forking thread
+  (`QorePythonDeclUse::countThreadUses()`, from the thread-local list of its uses): the uses of the parent's other
+  threads never end in the child, and deleting a Program would wait for them forever.  The handler reaches every
+  handle, including handles without an owner and handles already invalidated, through an intrusive list of all
+  handles (`DeclHandleRegistry::handles`), which needs no memory allocation in the child.
 - Qore's `fork()` called from Python code of the main interpreter (a Python process hosting Qore) runs Python's fork
   protocol, as `os.fork()` does: `callQoreFunction()` recognizes Qore's `fork()` and sets the thread-local
   `python_fork_protocol_tstate` to the thread state it releases for the call (`QorePythonForkProtocolHelper`), and

docs/guide-pythonreleasenotes.dox.tmpl

@@ -369,6 +369,15 @@
       previously the deletion waited forever for those threads, which do not exist in the child.  Such an interpreter
       is not finalized in the child if threads created by %Python in the parent had a state in it; it is left in
       memory.  New threads of the child that use an interpreter get their own %Python thread states.
+    - a child process created with \c fork() (@ref Qore::fork() "fork()" or \c os.fork()) on a system with the musl C
+      library (ex: Alpine Linux) can no longer hang in its first call from %Python to %Qore that releases the GIL, or
+      when it ends a use of a declaration of a %Qore @ref Qore::Program "Program" or deletes a %Python interpreter:
+      the condition variables of the module were copied to the child with the threads of the parent that waited on
+      them, which do not exist in the child, and musl waits for the threads that a condition wakes up
+    - a child process created with \c fork() while another thread used a class, hashdecl or enum of a %Qore
+      @ref Qore::Program "Program" from %Python (ex: while it ran the constructor of a %Qore class called from
+      %Python) can delete the %Program; previously the deletion waited forever for the use of that thread, which does
+      not exist in the child
     - @ref Qore::fork() "fork()" called from %Python code in a %Python process using %Qore through the \c qoreloader
       module runs %Python's fork protocol, as \c os.fork() does: the functions registered with
       \c os.register_at_fork() run, and the child no longer keeps the %Python state of the threads of the parent;

src/QorePythonDeclHandle.cpp

@@ -25,6 +25,7 @@
 #include <qore/QoreForkHandler.h>
 
 #include <map>
+#include <new>
 #include <optional>
 
 namespace {
@@ -40,6 +41,8 @@ struct DeclHandleRegistry {
     //! the number of threads waiting on cond
     unsigned waiting = 0;
     program_handle_map_t map;
+    //! the first handle in the list of all handles, which the child handler of the lock needs
+    QorePythonDeclHandle* handles = nullptr;
 };
 
 // not destroyed, so that handles of capsules freed while the process exits can still deregister
@@ -64,11 +67,26 @@ thread_local QorePythonDeclUse* QorePythonDeclUse::current = nullptr;
 int QorePythonDeclHandle::registerForkHandler() {
     assert(!decl_handle_fork_reg);
     decl_handle_fork_reg = qore_register_fork_handler(QFR_MODULE_LEAF, "python_qore_declaration_handles",
-        qore_fork_prepare_thread_lock, qore_fork_parent_thread_lock, qore_fork_child_thread_lock,
-        &get_registry().lck);
+        qore_fork_prepare_thread_lock, qore_fork_parent_thread_lock, forkChild, &get_registry().lck);
     return decl_handle_fork_reg ? 0 : -1;
 }
 
+void QorePythonDeclHandle::forkChild(void* lock) {
+    qore_fork_child_thread_lock(lock);
+    DeclHandleRegistry& registry = get_registry();
+    // the threads waiting in invalidate() do not exist in the child, and the child gets a new condition: its copy
+    // would still list them as waiters, and signaling it could wait for them to wake up (musl).  The old object is
+    // not destroyed, as pthread_cond_destroy() can also wait for its waiters (see QoreCondition::reinitAfterFork() in
+    // libqore, which modules cannot call)
+    registry.waiting = 0;
+    new (&registry.cond) QoreCondition;
+    // the uses of the other threads never end in the child: invalidate() would wait for them forever.  The memory
+    // of a handle whose destructor was waiting for the lock in another thread is not freed in the child
+    for (QorePythonDeclHandle* handle = registry.handles; handle; handle = handle->next_handle) {
+        handle->users = QorePythonDeclUse::countThreadUses(handle);
+    }
+}
+
 void QorePythonDeclHandle::unregisterForkHandler() {
     qore_unregister_fork_handler(decl_handle_fork_reg);
     decl_handle_fork_reg = nullptr;
@@ -77,19 +95,34 @@ void QorePythonDeclHandle::unregisterForkHandler() {
 std::shared_ptr<QorePythonDeclHandle> QorePythonDeclHandle::create(const void* decl, QoreProgram* owner, Kind kind,
         const char* name) {
     std::shared_ptr<QorePythonDeclHandle> rv(new QorePythonDeclHandle(decl, owner, kind, name));
+    DeclHandleRegistry& registry = get_registry();
+    AutoLocker al(registry.lck);
+    // linked first: if the map cannot be updated, the destructor unlinks the handle after the lock is released
+    rv->next_handle = registry.handles;
+    if (registry.handles) {
+        registry.handles->prev_handle = rv.get();
+    }
+    registry.handles = rv.get();
     if (owner) {
-        DeclHandleRegistry& registry = get_registry();
-        AutoLocker al(registry.lck);
         registry.map[owner].insert(handle_map_t::value_type(rv.get(), rv));
     }
     return rv;
 }
 
 QorePythonDeclHandle::~QorePythonDeclHandle() {
+    DeclHandleRegistry& registry = get_registry();
+    AutoLocker al(registry.lck);
     assert(!users);
+    if (prev_handle) {
+        prev_handle->next_handle = next_handle;
+    } else {
+        assert(registry.handles == this);
+        registry.handles = next_handle;
+    }
+    if (next_handle) {
+        next_handle->prev_handle = prev_handle;
+    }
     if (owner) {
-        DeclHandleRegistry& registry = get_registry();
-        AutoLocker al(registry.lck);
         // the entry has been removed if the Program has invalidated its handles
         program_handle_map_t::iterator i = registry.map.find(owner);
         if (i != registry.map.end()) {

src/QorePythonDeclHandle.h

@@ -41,7 +41,7 @@
     A declaration without an owning Program (one in the static system namespace) is never invalidated.
 
     The registry of handles and the uses of all handles are guarded by one leaf lock, which has fork handlers (see
-    registerForkHandler())
+    registerForkHandler()), which keep only the uses of the thread that called fork() in the child
 */
 class QorePythonDeclHandle {
     friend class QorePythonDeclUse;
@@ -117,6 +117,9 @@ class QorePythonDeclHandle {
 
     //! the number of current uses; guarded by the lock of the handles
     unsigned users = 0;
+    //! the previous and next handles in the list of all handles; guarded by the lock of the handles
+    QorePythonDeclHandle* prev_handle = nullptr;
+    QorePythonDeclHandle* next_handle = nullptr;
     //! false once the owning Program has deleted the declaration; written with the lock of the handles held
     std::atomic<bool> valid{true};
 
@@ -129,6 +132,12 @@ class QorePythonDeclHandle {
 
     //! ends a use started with startUse()
     DLLLOCAL void endUse();
+
+    //! the child handler of the lock of the handles
+    /** Only the thread that called fork() exists in the child: the uses and waits of the parent's other threads never
+        end in it
+    */
+    DLLLOCAL static void forkChild(void* lock);
 };
 
 //! A use of a declaration: while it is held, the declaration is not freed

src/QorePythonProgram.cpp

@@ -49,6 +49,7 @@
 #include <algorithm>
 #include <climits>
 #include <ctime>
+#include <new>
 #include <vector>
 #include <string>
 
@@ -135,6 +136,11 @@ void QorePythonProgram::forkChildThreadStates(void* arg) {
     for (auto& i : py_thr_map) {
         i.first->pgm_thr_cnt = 0;
         i.first->pgm_thr_waiting = 0;
+        // a new condition: the child's copy would still list the parent's threads waiting in waitForThreadsIntern()
+        // as waiters, and signaling it could wait for them to wake up (musl); the old object is not destroyed, as
+        // pthread_cond_destroy() can also wait for its waiters (see QoreCondition::reinitAfterFork() in libqore,
+        // which modules cannot call)
+        new (&i.first->pgm_thr_cond) QoreCondition;
         i.first->pgm_thr_uses.clear();
         i.first->pgm_cancelled_tids.clear();
     }

src/python-module.cpp

@@ -28,6 +28,7 @@
 #include <algorithm>
 #include <chrono>
 #include <condition_variable>
+#include <new>
 #include <pthread.h>
 #endif
 #include <cstring>
@@ -1452,6 +1453,11 @@ void gil_switch_fork_child() {
     if (gil_switch_fork_locked) {
         GilSwitchState& state = get_gil_switch_state();
         state.running = false;
+        // the child gets a new condition variable: the parent's helper thread may have been waiting on it, and the
+        // child's copy would still list that thread, which does not exist in the child, as a waiter; musl's
+        // pthread_cond_signal() waits for the waiters that it signals to wake up, so the next notify_one() would never
+        // return.  The old object is not destroyed, as pthread_cond_destroy() can also wait for its waiters
+        new (&state.cond) std::condition_variable;
         // only the thread that called fork() exists in the child
         unsigned long self = PyThread_get_thread_ident();
         for (size_t i = state.waiting.size(); i > 0; --i) {

test/docker_test/test-alpine.sh

@@ -41,6 +41,7 @@ PYTHONPATH="$PWD" python3 -B -W error ../test/standalone-threads.py
 PYTHONPATH="$PWD" python3 -B -W error ../test/standalone-exceptions.py
 PYTHONPATH="$PWD" python3 -B -W error ../test/standalone-objects.py
 PYTHONPATH="$PWD" python3 -B -W error ../test/standalone-signals.py
+PYTHONPATH="$PWD" python3 -B -W error ../test/standalone-fork.py
 PYTHONPATH="$PWD" python3 -B -W error ../test/standalone-exit.py
 PYTHONPATH="$PWD" python3 -B -W error ../test/standalone-loader.py
 

test/docker_test/test-ubuntu.sh

@@ -64,6 +64,7 @@ PYTHONPATH="$PWD" ${PYTHON} -B -W error ../test/standalone-threads.py
 PYTHONPATH="$PWD" ${PYTHON} -B -W error ../test/standalone-exceptions.py
 PYTHONPATH="$PWD" ${PYTHON} -B -W error ../test/standalone-objects.py
 PYTHONPATH="$PWD" ${PYTHON} -B -W error ../test/standalone-signals.py
+PYTHONPATH="$PWD" ${PYTHON} -B -W error ../test/standalone-fork.py
 PYTHONPATH="$PWD" ${PYTHON} -B -W error ../test/standalone-exit.py
 PYTHONPATH="$PWD" ${PYTHON} -B -W error ../test/standalone-loader.py