add Python 3.14 to CI

Author: oconnor663

.github/workflows/ci.yml

@@ -12,7 +12,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        python-version: ["3.9", "3.10", "3.11", "3.12", "3.13"]
+        python-version: ["3.9", "3.10", "3.11", "3.12", "3.13", "3.14"]
         os: [ubuntu-latest, windows-latest, macOS-latest]
 
     steps:

duct.py

@@ -57,6 +57,7 @@
 import signal
 import subprocess
 import threading
+import time
 
 try:
     from pathlib import PurePath
@@ -1249,61 +1250,36 @@ def __init__(self, *args, **kwargs):
         # blocked on wait().
         self._condvar = threading.Condition(threading.Lock())
 
-    def wait(self):
-        # Take the condvar/lock. If we need to do a blocking wait, we'll
-        # release it first.
+    def wait(self, timeout=None):
+        deadline = time.monotonic() + timeout if timeout is not None else None
+        # Start by taking the condvar/lock, but note that we need to release it
+        # before waiting, or else we'd block .poll(), .kill(), and other calls
+        # to .wait().
         with self._condvar:
-            # As long as another thread is in a blocking wait, sleep on the
-            # condvar. Once we wake from this sleep, it's overwhelmingly likely
-            # that the child will have been reaped, but we'll still check in
-            # case waiting fails somehow.
-            while self._someone_is_waiting:
-                self._condvar.wait()
-
-            # Check whether the child has already been reaped. (The .poll()
-            # below actually does the same check internally, so we could skip
-            # this, but it feels clearer to me to check it.)
-            if self._child.returncode is not None:
-                return self._child.returncode
-
-            # We have the condvar/lock, and we're the thread responsible for
-            # waiting on the child. We're about to release the lock and do a
-            # blocking, non-reaping wait. However, there's another subtle race
-            # condition we need to worry about, apart from the kill/wait PID
-            # reuse race that this class is all about. Here's a possible order
-            # of events:
-            #   1. The child process exits.
-            #   2. Lots of time passes. Now any waiter will certainly see that
-            #      the child has exited.
-            #   3. One thread calls .wait(). It acquires the lock, sets
-            #      _someone_is_waiting to True, and releases the lock, in
-            #      preparation for attempting its blocking wait.
-            #   4. Suddently, another thread swoops in and calls .poll().
-            #      That thread acquires the lock, sees _someone_is_waiting, and
-            #      returns None. ***THIS IS A BUG.***
-            #   5. The first thread sees that the child has exited, reacquires
-            #      the lock, and cleans up.
-            # If the call to .poll() were racing against the child's actual
-            # exit, then we wouldn't care what it returned. That would be an
-            # "honest" race, and it would be correct for the result to be a
-            # coin flip. But that's not what happens in this situation. Here
-            # the child has definitely exited, maybe seconds or minutes ago,
-            # and a single call to .poll() would certainly have returned
-            # non-None. It's only by racing against .wait() that this situation
-            # could incorrectly report that the child hasn't exited.
-            #
-            # To fix this, .wait() must do a non-blocking wait (that is,
-            # actually check the status of the child process) *before*
-            # releasing the lock. (In fact, doing it before acquiring the lock
-            # could also be correct.) If that returns false, then the only
-            # possible race is a race against the child itself, where again
-            # it's expected and fine for the result to be a coin flip.
-            if self._child.poll() is not None:
-                return self._child.returncode
+            while True:
+                now = time.monotonic()
+                if self._child.returncode is not None:
+                    # The child has already been reaped. Return its saved exit
+                    # status.
+                    return self._child.returncode
+                if deadline is not None and time.monotonic() > deadline:
+                    # The deadline has passed. Poll the child on the way out,
+                    # to make sure we always poll at least once.
+                    return self.poll()
+                if self._someone_is_waiting:
+                    # There is another blocking waiter. Sleep until it signals
+                    # the condvar or the deadline passes. Spurious wakeups are
+                    # acceptable here.
+                    condvar_timeout = deadline - now if deadline is not None else None
+                    self._condvar.wait(condvar_timeout)
+                else:
+                    # There are no other blocking waiters. Proceed to the
+                    # blocking wait.
+                    break
 
-            # Finally, before releasing the condvar/lock, set
-            # _someone_is_waiting. We must unset it before returning, or else
-            # we'll deadlock other callers.
+            # We are the blocking waiter. Set the _someone_is_waiting flag and
+            # release the lock (dedent) before blocking. After this, we must
+            # clear this flag and notify the condvar before returning.
             self._someone_is_waiting = True
 
         # Dedent to release the condvar/lock, and do the blocking wait. We have

gotchas.md

@@ -26,6 +26,7 @@ affect the implementation.
 * [Matching platform case-sensitivity for environment variables](#matching-platform-case-sensitivity-for-environment-variables)
 * [Using IO threads to avoid blocking children](#using-io-threads-to-avoid-blocking-children)
 * [Killing grandchild processes?](#killing-grandchild-processes)
+* [Waiting with a timeout](#waiting-with-a-timeout)
 
 ## Reporting errors by default
 
@@ -56,9 +57,10 @@ reading all of its input. Most standard libraries get this right.
 
 Notably on Unix, this requires the process to suppress `SIGPIPE`.
 Implementations in languages that don't suppress `SIGPIPE` by default (C/C++?)
-have no choice but to set a signal handler from library code, which might
-conflict with application code or other libraries. There is no good solution to
-this problem.
+have to configure signal handling from library code, which might conflict with
+application code or other libraries in the rare case that something does want
+to receive that signal. (See [Waiting with a timeout](#waiting-with-a-timeout)
+below for more on handling signals from library code.)
 
 ## Cleaning up zombie children
 
@@ -100,8 +102,7 @@ PID. It's not likely, but all of that could happen just before the call to
 is why the Rust standard library [doesn't allow shared access to child
 processes](https://doc.rust-lang.org/std/process/struct.Child.html#method.kill).
 
-It's possible to avoid this race using a newer POSIX API called
-[`waitid`](https://pubs.opengroup.org/onlinepubs/9699919799/functions/waitid.html).
+It's possible to avoid this race using a newer POSIX API called [`waitid`].
 That function has a `WNOWAIT` flag that leaves the child in its zombie state,
 so that its PID isn't freed for reuse. That gives the waiting thread a chance
 to set a flag to block further kills, before reaping the child. Duct uses this
@@ -306,3 +307,41 @@ objects](https://docs.microsoft.com/en-us/windows/win32/procthread/job-objects))
 but even there it sounds like some important features aren't supported on
 Windows 7. Realistically, there won't be good techniques for Duct to use to
 solve this problem for many years.
+
+## Waiting with a timeout
+
+The Windows [`WaitForSingleObject`] function has a timeout argument, but the
+Unix [`waitpid`], [`waitid`], and [`pthread_join`] functions do not. That makes
+it complicated to do any sort of waiting with a timeout on Unix.
+
+- for threads we can add code
+  - what does Python do?
+    - PyEvent on thread exit
+    - also their locks are actually all condvars on the inside and support
+      waiting with a timeout
+- for children we need to handle SIGCHLD
+  - what does Python do? (timeout?)
+    - OMG does Python have the wait/try_wait race?!
+  - signal_hook_registry race condition
+    - can we just have sigaction() write back to the global?
+  - does Rust Child::wait fail to handle eintr?
+
+
+I want try_wait/poll to always actually check, because you might be calling it
+in response to SIGCHLD or something like that, and in that case it's
+unacceptable for a race against the blocking thread to cause you to return
+None.
+
+That means that all reaping should actually be done by calling into wait().
+
+That means that double locking is viable, like the old Python implementation.
+Maybe we don't need a condvar?
+- But what does that mean for timeouts? What's cleaner? Unix has SIGCHLD, and
+  Windows doesn't have to worry about reaping, but what would we need to do if
+  there was a timeout parameter to the Unix waitid? In that case we would need
+  the condvar.
+
+[`WaitForSingleObject`]: https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-waitforsingleobject
+[`waitpid`]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/waitpid.html
+[`waitid`]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/waitid.html
+[`pthread_join`]: https://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_join.html