test: Clean shutdown in Socks5Server #34863

optout21 commented at 9:32 AM on March 19, 2026: contributor

Add clean shutdown to Socks5Server test utility, fix a sporadic CI failure.

The Socks5Server utility handles multiple incoming connections, which are handled in separate background threads. Its stop() method unblocks and waits for the main background thread cleanly, but it doesn't attempt to wait for the completion of handler threads. There is no guarantee that the handler threads are finished after stop() returns, which can lead to IO errors.

In the reported sporadic test failures, the test in p2p_private_broadcast.py uses the Socks5 server, and makes a node connect to/through it. Then it stops the Socks5 server, and then it stops the node. However, if a connection handler is still active, that can lead to errors, as the socket is closed.

The change attempts to fix this by storing all handler threads and connections, and attempting to shut them down before stop() returns.

Notes:

~~I was not able to reliably reproduce the failure locally.~~ Update: I could reproduce the failure, see #34849 (comment) .

Running the relevant test:

build/test/functional/test_runner.py p2p_private_broadcast.py

DrahtBot added the label Tests on Mar 19, 2026

DrahtBot commented at 9:33 AM on March 19, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	vasild, andrewtoth, w0xlt, achow101

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

optout21 renamed this:
~~test: Clean shutdown in Socks5Server (CI failure)~~
test: Clean shutdown in Socks5Server (CI fix)
on Mar 19, 2026

optout21 closed this on Mar 20, 2026

optout21 reopened this on Mar 20, 2026

fanquake renamed this:
~~test: Clean shutdown in Socks5Server (CI fix)~~
test: Clean shutdown in Socks5Server
on Mar 20, 2026

fanquake commented at 4:43 AM on March 23, 2026: member

cc @vasild

optout21 renamed this:
~~test: Clean shutdown in Socks5Server~~
test: CI fix: Clean shutdown in Socks5Server
on Mar 23, 2026

in test/functional/test_framework/socks5.py:113 in 355f175c13 outdated

 112 | @@ -113,6 +113,8 @@ class Socks5Connection():
 113 |      def __init__(self, serv, conn):

vasild commented at 3:19 PM on March 24, 2026:

nit: might prefix the commit message's first line with test:

optout21 commented at 1:50 PM on March 25, 2026:

done

in test/functional/test_framework/socks5.py:274 in 355f175c13

 265 | @@ -240,3 +266,14 @@ def stop(self):
 266 |          s.connect(self.conf.addr)
 267 |          s.close()
 268 |          self.thread.join()
 269 | +        # if there are active handler, close them
 270 | +        with self._handlers_lock:
 271 | +            items = list(self.handlers)
 272 | +        for thread, conn in items:
 273 | +            # check if thread is still active
 274 | +            thread.join(timeout=0)

vasild commented at 3:36 PM on March 24, 2026:

https://docs.python.org/3/library/threading.html#meth-thread-join

If an attempt is made to join a running daemonic thread in late stages of Python finalization join() raises a PythonFinalizationError.

Our threads are "demonic" because we do thread.daemon = True when creating them.

optout21 commented at 1:33 PM on March 25, 2026:

Good point. Since the exception only may occur in late stages of finalization, I think the correct handling is to wrap into a try, and ignore the exception. I do this in a helper method now.

vasild commented at 12:47 PM on March 30, 2026:

Or make the threads non-demonic. I think originally they were made like that so that we do not have to care about collecting the resources after they exit. But now with the change in this PR we will wait for the threads and join them. I see no reason for them to be damonic, I might be missing something though...

optout21 commented at 1:50 PM on March 30, 2026:

I'm not sure what the intent of using daemonic threads was, they can continue running even after the main thread exits, but I don't see a need for that. With this change all threads would be joined during stop. I think non-daemonic threads would be a cleaner solution. I will check into it.

in test/functional/test_framework/socks5.py:209 in 355f175c13 outdated

 205 |                  logger.debug("Keeping client connection alive")
 206 |  
 207 | +    def force_close(self):
 208 | +        # close underlying connections
 209 | +        try:
 210 | +            self.conn.close()

vasild commented at 3:39 PM on March 24, 2026:

The new method force_close() will be enough to cancel the loop in forward_sockets() because it closes the sockets, but I think it may end up raise IOError('Exceptional condition on socket'), not a graceful exit.

optout21 commented at 1:36 PM on March 25, 2026:

The same exception can occur without force_close(), and it is handled in an except in handle(). If the proxy is not stopped (or stopped 'half-way' with the current stop()), and the connection initiator or receiver closes the connection (e.g. as the node is stopped), the same exception occurs (socked closed while in a blocking read), and it is handled. So using the force_close() triggers the same exception, but it does so before stop() returns.

in test/functional/test_framework/socks5.py:186 in 355f175c13

 183 | @@ -182,6 +184,7 @@ def handle(self):
 184 |                      logger.debug(f"Serving connection to {requested_to}, will redirect it to "
 185 |                                   f"{dest['actual_to_addr']}:{dest['actual_to_port']} instead")
 186 |                      with socket.create_connection((dest["actual_to_addr"], dest["actual_to_port"])) as conn_to:

vasild commented at 3:42 PM on March 24, 2026:

comment in arbitrary location

I think it would be better to set self.running to False (already done in stop()) and check for that in forward_sockets() and when it becomes False then exist gracefully from forward_sockets(). Then in stop() wait indefinitely for handler threads to exit by themselves (they will since they will be checking self.serv.running. Maybe that running flag would need a mutex.

optout21 commented at 1:47 PM on March 25, 2026:

forward_sockets() does a blocking select & read on the sockets, in a loop. If the sockets exist, but not yet closed nor written to, it will block there. While blocking, it cannot check for the running flag. If we want to make sure that when stop exists, the handling threads exit, we need to unblock them. Just waiting could cause a deadlock. Another way would be to have a special socket, and include that in the select call, and use that to unblock, to give a chance for checking the stop flag. The same is achieved by closing the sockets, which may cause IOException, but I don't think that's a problem, as described earlier. I'm open to using a special signaling socket, if you prefer.

I did add an extra check for running before calling forward_sockets().

As for the mutex, I added it (though I think setting/gettin a single bool value is Python is fine).

vasild commented at 2:15 PM on March 30, 2026:

hmm, wait, forward_sockets() does setblocking(False) on both sockets. So the read is nonblocking. The select is without a specified timeout - until something happens. I think it would be cleanest if a timeout of e.g. 500ms is provided to select and then the forward_sockets() function checks the "is running" flag on every iteration of the loop.

optout21 commented at 4:10 PM on April 7, 2026:

I've added a wake up pipe to the select, so now it can be unblocked at shutdown. The loop exits if running became false. I've added a timeout as well, but just for robustness, it's not really needed (2 sec timeout).

optout21 commented at 12:48 PM on March 25, 2026: contributor

Update: I've managed to force-reproduce the issue, and the present fix appears to indeed fix the problem. For details, see: #34849 (comment) @vasild , thanks for the review, I will address them soon.

optout21 force-pushed on Mar 25, 2026

optout21 commented at 10:04 PM on March 25, 2026: contributor

Applied changed following review comments:

Guard against potential exception in thread.join(), that can occur with daemonic threads
Add an extra check on running before calling forward_sockets()
Protect the running flag by a lock

in test/functional/test_framework/socks5.py:198 in bf7fc3d865

 203 | +                    logger.debug(f"Can't serve the connection to {requested_to}: no destinations factory")
 204 |  
 205 |              # Fall through to disconnect
 206 |          except Exception as e:
 207 |              logger.exception("socks5 request handling failed.")
 208 |              self.serv.queue.put(e)

andrewtoth commented at 12:05 AM on March 30, 2026:

Don't we want to guard here against adding in exceptions that are thrown from a force close?

            if self.serv.is_running():
                self.serv.queue.put(e)

optout21 commented at 2:30 PM on March 30, 2026:

Taken. In the previous version, this was the case, but then I removed the if. But you are right, it's better to ignore the post-stop exceptions here. I've also added printing of the running flag in the log line above.

in test/functional/test_framework/socks5.py:215 in bf7fc3d865

 220 | +            logger.debug("Incoming connection force-closed")
 221 | +        except OSError:
 222 | +            pass
 223 | +        if self.conn_to:
 224 | +            try:
 225 | +                self.conn_to.close()

andrewtoth commented at 12:11 AM on March 30, 2026:

Possible race here, if self.conn_to is set to None above in the handler's finally block. If the handler finishes right after we enter this if, we will call close() on a None and raise an AttributeError. Recommend assigning conn_to = self.conn_to before the if above:

    conn_to = self.conn_to
    if conn_to:
        try:
            conn_to.close()

optout21 commented at 2:30 PM on March 30, 2026:

Done.

in test/functional/test_framework/socks5.py:224 in bf7fc3d865

 229 | +                pass
 230 | +
 231 | +
 232 | +# Wrapper for thread.join(), which may throw for daemonic threads (in late stages of finalization)
 233 | +# https://docs.python.org/3/library/threading.html#meth-thread-join
 234 | +def is_daemonic_thread_alive(thread, timeout=0) -> bool:

andrewtoth commented at 12:12 AM on March 30, 2026:

This name is not that descriptive IMO. Maybe something like try_join_daemonic_thread?

optout21 commented at 2:30 PM on March 30, 2026:

Indeed, was wrong name, changed to try_join_daemon_thread

in test/functional/test_framework/socks5.py:254 in bf7fc3d865 outdated

 250 |          self.thread = None
 251 |          self.queue = queue.Queue() # report connections and exceptions to client
 252 |          self.keep_alive = conf.keep_alive
 253 | +        # Store the background handlers, needed for clean shutdown
 254 | +        self.handlers = []
 255 | +        self._handlers_lock = threading.Lock()

vasild commented at 1:22 PM on March 30, 2026:

There is some inconsistency with the naming:

_running
_running_lock

handlers // hmm, no _ prefix?
_handlers_lock

optout21 commented at 2:31 PM on March 30, 2026:

I think here the _ suggests privateness. _running has an accessor, but handlers is accessed directly from outside. Leaving as it is.

vasild commented at 10:23 AM on April 7, 2026:

If handlers is accessed outside, then _handlers_lock must also be accessed from outside, so, that should be handlers_lock?

optout21 commented at 4:05 PM on April 7, 2026:

Actually, both are accessed only from within the class Socks5Server, so i changed them both with underscore prefix.

in test/functional/test_framework/socks5.py:289 in bf7fc3d865

 290 |          # connect to self to end run loop
 291 |          s = socket.socket(self.conf.af)
 292 |          s.connect(self.conf.addr)
 293 |          s.close()
 294 |          self.thread.join()
 295 | +        # if there are active handler, close them

vasild commented at 1:23 PM on March 30, 2026:

        # if there are active handlers, close them

in test/functional/test_framework/socks5.py:297 in bf7fc3d865

 298 | +        for thread, conn in items:
 299 | +            # check if thread is still active
 300 | +            if is_daemonic_thread_alive(thread):
 301 | +                conn.force_close()
 302 | +                if is_daemonic_thread_alive(thread, timeout=2):
 303 | +                    logger.warning("Stop(): A handler thread didn't finish after closing the sockets")

vasild commented at 1:53 PM on March 30, 2026:

What is the expected flow with this change during shutdown? Why would a thread have exited on line 294? Would it always enter that if because the read is always alive? I would expect something like:

Do something to make threads exit, like raising a flag or closing their sockets
Wait indefinitely for them to exist

Now, that 1. could be a boolean flag, but as you mentioned the threads are in a blocking read, so better close the sockets. So maybe this can be simplified to:

for thread, conn in items:
    conn.force_close()
    thread.join()

and make the threads non-daemonic.

optout21 commented at 2:36 PM on March 30, 2026:

Processing threads may exit 'normally', when their client closes the connection. It's normal for one proxy to have multiple subsequent connections that are closed. Processing thread handles are added to the handlers array, but not removed. So it can happen that by the time of stop, some thread handler are defunct (has been exited already). The proper way to check for these is to do a join with 0 timeout and call is_active(). Closing the sockets for already-exited threads is not needed and may cause issues.

vasild commented at 10:32 AM on April 7, 2026:

Got it. Thanks for the explanation.

Processing thread handles are added to the handlers array, but not removed

Hmm, so they pile up forever in the array. I guess this is not an issue in our case because we do not have tests that run for long time and serve clients "indefinitely". Still, it would be cleaner if somehow the array is purged. Could an exiting thread remove itself just before exiting?

optout21 commented at 4:04 PM on April 7, 2026:

Implemented removing a handler after it finishes normally, so they are not kept until the very end.

in test/functional/test_framework/socks5.py:188 in bf7fc3d865 outdated

 190 | +                    dest = self.serv.conf.destinations_factory(requested_to_addr, port)
 191 | +                    if dest is not None:
 192 | +                        logger.debug(f"Serving connection to {requested_to}, will redirect it to "
 193 | +                                    f"{dest['actual_to_addr']}:{dest['actual_to_port']} instead")
 194 | +                        with socket.create_connection((dest["actual_to_addr"], dest["actual_to_port"])) as conn_to:
 195 | +                            self.conn_to = conn_to

vasild commented at 1:55 PM on March 30, 2026:

I think it would be enough to close only self.conn in order to get forward_sockets() to quit. If that is the case then no need to save also conn_to.

optout21 commented at 2:39 PM on March 30, 2026:

You are probably right, forward_sockets() selects on both, so closing the incoming socket is enough to wake it up. However, I think it's still a good idea to also close the outgoing socket:

to give a notification to the other reading end of the outgoing socket (that party doesn't know about the incoming socket of the proxy)
trigger unblock in the case the incoming socket has been closed already (I'm not sure if this can happen, though)

I did notice that the outgoing socket is never closed, I've added that to the normal flow as well now.

optout21 commented at 8:20 AM on April 8, 2026:

The sockets are not force-closed any more, only the forwarding is stopped.

optout21 force-pushed on Mar 30, 2026

optout21 commented at 2:43 PM on March 30, 2026: contributor

Applied changes following review comments (thanks @andrewtoth @vasild ). One issue still pending: don't use daemon threads. Update: daemon thread usage was removed, then reverted.

optout21 force-pushed on Mar 31, 2026

optout21 commented at 8:31 AM on March 31, 2026: contributor

Applied change to use regular threads instead of daemon threads, as suggested (@vasild). Regular threads are sufficient and allow for nicer cleanup. Also note that I have not found any other usage of daemon threads in the project. Update: this change was reverted.

optout21 force-pushed on Mar 31, 2026

DrahtBot added the label CI failed on Mar 31, 2026

optout21 commented at 11:11 AM on March 31, 2026: contributor

The test feature_proxy.py failed with the non-daemon-thread change, it was lacking a stop on the proxy. Fix applied: stop added. Update: this change was reverted.

maflcko commented at 7:39 AM on April 1, 2026: member

Looks like it times out on Windows?

optout21 commented at 8:59 AM on April 1, 2026: contributor

Looks like it times out on Windows?

It looks so, and it it probably caused by my last change -- not using daemon threads (unless tests hung due to some other reason, which is unlikely). The last change was to move away from using special "daemon" threads. This caused a hanging functional test (feature_proxy.py), CI timed out, and could be also reliably seen in local execution. I've fixed this by adding missing shutdown in the test. That corrected the test in local run, and in most CI runs. but apparently the Windows CI time out. I can't see until which test is got. To be on the safe side and for simplicity I will revert the last change, and use deamon thread as originally.

optout21 commented at 9:07 AM on April 1, 2026: contributor

The last change -- not using daemon threads -- was reverted, due to CI failures (on Windows). (More precisely, last two changes: switch to using regular threads, and a subsequent fix for feature_proxy.py.)

optout21 force-pushed on Apr 1, 2026

DrahtBot removed the label CI failed on Apr 1, 2026

achow101 added this to the milestone 31.0 on Apr 1, 2026

achow101 commented at 2:15 AM on April 4, 2026: member

ACK 778388a64fe07e7678a1c303a4e2d6e28ca956c8

andrewtoth approved

andrewtoth commented at 2:22 PM on April 4, 2026: contributor

ACK 778388a64fe07e7678a1c303a4e2d6e28ca956c8

in test/functional/test_framework/socks5.py:231 in 778388a64f outdated

 238 | +# Return True if thread is still alive.
 239 | +# See PR #34863 for more details on using daemon threads.
 240 | +def try_join_daemon_thread(thread, timeout=0) -> bool:
 241 | +    try:
 242 | +        thread.join(timeout=timeout)
 243 | +        return thread.is_alive()

rkrux commented at 9:48 AM on April 6, 2026:

Return True if thread is still alive.

Reading if try_join_daemon_thread(): at first gives the impression that the thread joined successfully, which implies that the thread should not be alive any longer. I feel it should return false if the thread is still alive.

vasild commented at 10:38 AM on April 7, 2026:

Yeah - "try_join() -> true" means that "it joined".

-        return thread.is_alive()
+        return !thread.is_alive()

optout21 commented at 3:54 PM on April 7, 2026:

Accepted, makes sense, I've reverted the semantics of the return value.

in test/functional/test_framework/socks5.py:275 in 778388a64f outdated

 273 | -            if self.running:
 274 | +            if self.is_running():
 275 |                  conn = Socks5Connection(self, sockconn)
 276 |                  thread = threading.Thread(None, conn.handle)
 277 | +                # Use "daemon" threads, see PR #34863 for more discussion.
 278 |                  thread.daemon = True

rkrux commented at 9:50 AM on April 6, 2026:

Nit for succinctness:

@@ -270,9 +270,8 @@ class Socks5Server():
             (sockconn, _) = self.s.accept()
             if self.is_running():
                 conn = Socks5Connection(self, sockconn)
-                thread = threading.Thread(None, conn.handle)
                 # Use "daemon" threads, see PR [#34863](/bitcoin-bitcoin/34863/) for more discussion.
-                thread.daemon = True
+                thread = threading.Thread(None, conn.handle, daemon=True)
                 with self._handlers_lock:
                     self.handlers.append((thread, conn))
                 thread.start()

optout21 commented at 3:55 PM on April 7, 2026:

Done, though this is not a necessary change, and touches unmodified lines.

rkrux commented at 9:50 AM on April 6, 2026: contributor

I see the following errors on my system (macOS) but the test passes.

<details open> <summary>Test error Logs</summary>

2026-04-06T09:36:29.134544Z TestFramework (INFO): Checking abortprivatebroadcast fails for non-existent transaction
2026-04-06T09:36:29.135831Z TestFramework.socks5 (ERROR): socks5 request handling failed (running False)
Traceback (most recent call last):
  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/socks5.py", line 189, in handle
    forward_sockets(self.conn, conn_to)
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^
  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/socks5.py", line 64, in forward_sockets
    rlist, _, xlist = select.select(sockets, [], sockets)
                      ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
OSError: [Errno 9] Bad file descriptor
2026-04-06T09:36:29.139661Z TestFramework.socks5 (ERROR): socks5 request handling failed (running False)
Traceback (most recent call last):
  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/socks5.py", line 189, in handle
    forward_sockets(self.conn, conn_to)
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^
  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/socks5.py", line 64, in forward_sockets
    rlist, _, xlist = select.select(sockets, [], sockets)
                      ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
OSError: [Errno 9] Bad file descriptor
2026-04-06T09:36:29.141264Z TestFramework.socks5 (ERROR): socks5 request handling failed (running False)
Traceback (most recent call last):
  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/socks5.py", line 189, in handle
    forward_sockets(self.conn, conn_to)
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^
  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/socks5.py", line 64, in forward_sockets
    rlist, _, xlist = select.select(sockets, [], sockets)
                      ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
OSError: [Errno 9] Bad file descriptor
2026-04-06T09:36:29.142593Z TestFramework.socks5 (ERROR): socks5 request handling failed (running False)
Traceback (most recent call last):
  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/socks5.py", line 189, in handle
    forward_sockets(self.conn, conn_to)
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^
  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/socks5.py", line 64, in forward_sockets
    rlist, _, xlist = select.select(sockets, [], sockets)
                      ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
OSError: [Errno 9] Bad file descriptor
2026-04-06T09:36:29.143793Z TestFramework.socks5 (ERROR): socks5 request handling failed (running False)
Traceback (most recent call last):
  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/socks5.py", line 189, in handle
    forward_sockets(self.conn, conn_to)
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^
  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/socks5.py", line 64, in forward_sockets
    rlist, _, xlist = select.select(sockets, [], sockets)
                      ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
OSError: [Errno 9] Bad file descriptor
2026-04-06T09:36:29.145172Z TestFramework.socks5 (ERROR): socks5 request handling failed (running False)
Traceback (most recent call last):
  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/socks5.py", line 189, in handle
    forward_sockets(self.conn, conn_to)
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^
  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/socks5.py", line 64, in forward_sockets
    rlist, _, xlist = select.select(sockets, [], sockets)
                      ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
OSError: [Errno 9] Bad file descriptor
2026-04-06T09:36:29.146707Z TestFramework.socks5 (ERROR): socks5 request handling failed (running False)
Traceback (most recent call last):
  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/socks5.py", line 189, in handle
    forward_sockets(self.conn, conn_to)
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^
  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/socks5.py", line 64, in forward_sockets
    rlist, _, xlist = select.select(sockets, [], sockets)
                      ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
OSError: [Errno 9] Bad file descriptor
2026-04-06T09:36:29.148110Z TestFramework.socks5 (ERROR): socks5 request handling failed (running False)
Traceback (most recent call last):
  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/socks5.py", line 189, in handle
    forward_sockets(self.conn, conn_to)
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^
  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/socks5.py", line 64, in forward_sockets
    rlist, _, xlist = select.select(sockets, [], sockets)
                      ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
OSError: [Errno 9] Bad file descriptor
2026-04-06T09:36:29.149317Z TestFramework.socks5 (ERROR): socks5 request handling failed (running False)
Traceback (most recent call last):
  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/socks5.py", line 189, in handle
    forward_sockets(self.conn, conn_to)
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^
  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/socks5.py", line 64, in forward_sockets
    rlist, _, xlist = select.select(sockets, [], sockets)
                      ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
OSError: [Errno 9] Bad file descriptor
2026-04-06T09:36:29.150613Z TestFramework.socks5 (ERROR): socks5 request handling failed (running False)
Traceback (most recent call last):
  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/socks5.py", line 189, in handle
    forward_sockets(self.conn, conn_to)
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^
  File "/Users/rkrux/projects/bitcoin/test/functional/test_framework/socks5.py", line 64, in forward_sockets
    rlist, _, xlist = select.select(sockets, [], sockets)
                      ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
OSError: [Errno 9] Bad file descriptor
2026-04-06T09:36:29.151722Z TestFramework (INFO): Trying to send a transaction when none of Tor or I2P is reachable
2026-04-06T09:36:29.940576Z TestFramework (INFO): Stopping nodes

</details>

vasild approved

vasild commented at 10:44 AM on April 7, 2026: contributor

ACK 778388a64fe07e7678a1c303a4e2d6e28ca956c8

I think the errors reported in #34863#pullrequestreview-4061330960 are to be expected with this PR, but (if that is the case), then maybe it would be better to silence them because they will keep scaring people, giving the impression that something is wrong.

rkrux commented at 11:15 AM on April 7, 2026: contributor

the errors reported in #34863#pullrequestreview-4061330960 are to be expected with this PR, but (if that is the case), then maybe it would be better to silence them

If these are expected, then it makes sense to silence them.

because they will keep scaring people, giving the impression that something is wrong.

True, they will gather unnecessary attention otherwise.

fanquake commented at 1:51 PM on April 7, 2026: member

Yea; "passing" tests shouldn't produce spurious (and undocumented) error output.

optout21 marked this as a draft on Apr 7, 2026

DrahtBot added the label CI failed on Apr 7, 2026

optout21 force-pushed on Apr 7, 2026

optout21 marked this as ready for review on Apr 7, 2026

optout21 marked this as a draft on Apr 7, 2026

optout21 force-pushed on Apr 7, 2026

optout21 commented at 4:01 PM on April 7, 2026: contributor

To mitigate the 'harmless' errors during shutdown, implemented more elaborate shutdown. As the CI failure is not that critical, there is time to improve the solution :) . A pipeline is used to wake up blocking select, add a timeout was also added to the select. Thus there is no need to force close underlying sockets.

Changes done:

Use a pipe to wake up blocking select, don't force close underlying sockets, add timeout to select, revert storing of conn_to.
Revert the semantics of the return value of try_join_daemon_thread
When a handler completes normally, remove if from the stored handlers; change handlers to dictionary @rkrux , thanks for the log, and I'd appreciate if you could retest on MacOS.

optout21 marked this as ready for review on Apr 7, 2026

achow101 commented at 5:52 PM on April 7, 2026: member

Some Windows CI is failing

  test  2026-04-07T16:39:53.047054Z TestFramework.socks5 (DEBUG): Serving connection to testonlyal77777777777777777777777777777777777777777vp6qd.onion:8333, will redirect it to 127.0.0.1:53307 instead 
 test  2026-04-07T16:39:53.047907Z TestFramework.socks5 (ERROR): socks5 request handling failed (running True) 
                                   Traceback (most recent call last):
                                     File "D:\a\bitcoin\bitcoin\build\test\functional\test_framework\socks5.py", line 195, in handle
                                       forward_sockets(self.conn, conn_to, self.wakeup_pipe[1], self.serv)
                                       ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                                     File "D:\a\bitcoin\bitcoin\build\test\functional\test_framework\socks5.py", line 66, in forward_sockets
                                       rlist, _, xlist = select.select(sockets, [], sockets, 2)
                                                         ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^
                                   OSError: [WinError 10038] An operation was attempted on something that is not a socket

w0xlt commented at 9:27 PM on April 7, 2026: contributor

According to the Python docs:

Return a pair of file descriptors (r, w) usable for reading and writing, respectively.

The wake-up path is reversed here: os.pipe() returns (read_fd, write_fd), but forward_sockets() waits on self.wakeup_pipe[1] and force_close() writes to self.wakeup_pipe[0].

--- a/test/functional/test_framework/socks5.py
+++ b/test/functional/test_framework/socks5.py
@@ -192,7 +192,7 @@ class Socks5Connection():
                         logger.debug(f"Serving connection to {requested_to}, will redirect it to "
                                     f"{dest['actual_to_addr']}:{dest['actual_to_port']} instead")
                         with socket.create_connection((dest["actual_to_addr"], dest["actual_to_port"])) as conn_to:
-                            forward_sockets(self.conn, conn_to, self.wakeup_pipe[1], self.serv)
+                            forward_sockets(self.conn, conn_to, self.wakeup_pipe[0], self.serv)
                             conn_to.close()
                     else:
                         logger.debug(f"Can't serve the connection to {requested_to}: the destinations factory returned None")
@@ -224,7 +224,7 @@ class Socks5Connection():
         # Wake up the blocking forwarding select by writing to the wake-up pipe
         try:
             if self.wakeup_pipe:
-                os.write(self.wakeup_pipe[0], "CloseWakeup".encode())
+                os.write(self.wakeup_pipe[1], "CloseWakeup".encode())

optout21 marked this as a draft on Apr 7, 2026

optout21 commented at 9:34 PM on April 7, 2026: contributor

Some Windows CI is failing

The Python select.select() call takes file descriptors (files, socket, pipes), but apparently on Windows it works only with sockets. Changing to use a pair of connected sockets (from socket.socketpair()) instead of a pipe.

Ref.: https://stackoverflow.com/questions/20952888/select-on-multiple-pipes "File objects on Windows are not acceptable, but sockets are. On Windows, the underlying select() function is provided by the WinSock library, and does not handle file descriptors that don’t originate from WinSock."

w0xlt commented at 9:34 PM on April 7, 2026: contributor

Alternatively os.pipe() wakeup FD can be repalced with socket.socketpair(), so forward_sockets() now passes only sockets to select(), which is required on Windows. The wakeup socket is then handled as a shutdown signal rather than forwarded traffic.

diff --git a/test/functional/test_framework/socks5.py b/test/functional/test_framework/socks5.py
index c644eaa39c..22d7eec1eb 100644
--- a/test/functional/test_framework/socks5.py
+++ b/test/functional/test_framework/socks5.py
@@ -4,7 +4,6 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Dummy Socks5 server for testing."""
 
-import os
 import select
 import socket
 import threading
@@ -71,6 +70,9 @@ def forward_sockets(a, b, wakeup_socket, serv):
             raise IOError('Exceptional condition on socket')
         for s in rlist:
             data = s.recv(4096)
+            if s == wakeup_socket:
+                logger.debug("forward_sockets: Exit due to wakeup")
+                return
             if data is None or len(data) == 0:
                 done = True
                 break
@@ -118,8 +120,8 @@ class Socks5Connection():
     def __init__(self, serv, conn):
         self.serv = serv
         self.conn = conn
-        # Pipe used to wake up blocking forwarding select
-        self.wakeup_pipe = os.pipe()
+        # Socketpair used to wake up blocking forwarding select on all platforms.
+        self.wakeup_sockets = socket.socketpair()
         # Index of this handler (within the server)
         self.handler_index = None
 
@@ -192,7 +194,7 @@ class Socks5Connection():
                         logger.debug(f"Serving connection to {requested_to}, will redirect it to "
                                     f"{dest['actual_to_addr']}:{dest['actual_to_port']} instead")
                         with socket.create_connection((dest["actual_to_addr"], dest["actual_to_port"])) as conn_to:
-                            forward_sockets(self.conn, conn_to, self.wakeup_pipe[1], self.serv)
+                            forward_sockets(self.conn, conn_to, self.wakeup_sockets[1], self.serv)
                             conn_to.close()
                     else:
                         logger.debug(f"Can't serve the connection to {requested_to}: the destinations factory returned None")
@@ -209,22 +211,22 @@ class Socks5Connection():
                 self.conn.close()
             else:
                 logger.debug("Keeping client connection alive")
-            if self.wakeup_pipe:
-                try:
-                    os.close(self.wakeup_pipe[0])
-                    os.close(self.wakeup_pipe[1])
-                except OSError:
-                    pass
-                self.wakeup_pipe = None
+            if self.wakeup_sockets:
+                for wakeup_socket in self.wakeup_sockets:
+                    try:
+                        wakeup_socket.close()
+                    except OSError:
+                        pass
+                self.wakeup_sockets = None
             if self.handler_index:
                 self.serv.remove_handler(self.handler_index)
                 self.handler_index = None
 
     def force_close(self):
-        # Wake up the blocking forwarding select by writing to the wake-up pipe
+        # Wake up the blocking forwarding select by writing to the wake-up socket.
         try:
-            if self.wakeup_pipe:
-                os.write(self.wakeup_pipe[0], "CloseWakeup".encode())
+            if self.wakeup_sockets:
+                self.wakeup_sockets[0].sendall(b"CloseWakeup")
                 logger.debug("Waking up forwarding thread")
         except OSError as e:
             logger.warning(f"Error waking up forwarding thread: {e}")

optout21 commented at 9:41 PM on April 7, 2026: contributor

Alternatively os.pipe() wakeup FD can be repalced with socket.socketpair()

Thanks, I just did exactly that :)

optout21 commented at 9:45 PM on April 7, 2026: contributor

The wake-up path is reversed here: os.pipe() returns (read_fd, write_fd), but forward_sockets() waits on self.wakeup_pipe[1] and force_close() writes to self.wakeup_pipe[0].

Good observation, I did it reversed, but still worked. Anyways, it's irrelevant now, as pipe is no longer used. The failure on Windows had a different reason.

optout21 force-pushed on Apr 7, 2026

DrahtBot removed the label CI failed on Apr 7, 2026

w0xlt commented at 11:32 PM on April 7, 2026: contributor

It is better to assign each SOCKS5 handler a unique, ever-increasing ID instead of basing it on the current number of active handlers. This avoids reusing IDs after out-of-order cleanup, which could overwrite a live handler.

diff --git a/test/functional/test_framework/socks5.py b/test/functional/test_framework/socks5.py
index cef70bbcca..31b8a027cf 100644
--- a/test/functional/test_framework/socks5.py
+++ b/test/functional/test_framework/socks5.py
@@ -265,6 +265,8 @@ class Socks5Server():
         # Store the background handlers, needed for clean shutdown
         self._handlers = {}
         self._handlers_lock = threading.Lock()
+        # Allocate handler IDs monotonically so active entries are never reused.
+        self._next_handler_index = 0
 
     def is_running(self) -> bool:
         with self._running_lock:
@@ -282,7 +284,8 @@ class Socks5Server():
                 # Use "daemon" threads, see PR [#34863](/bitcoin-bitcoin/34863/) for more discussion.
                 thread = threading.Thread(None, conn.handle, daemon=True)
                 with self._handlers_lock:
-                    conn.handler_index = len(self._handlers)
+                    conn.handler_index = self._next_handler_index
+                    self._next_handler_index += 1
                     self._handlers[conn.handler_index] = (thread, conn)
                 thread.start()

in test/functional/test_framework/socks5.py:219 in 367ffd1325

 226 | +                    self.wakeup_socket_pair[0].close()
 227 | +                    self.wakeup_socket_pair[1].close()
 228 | +                except OSError:
 229 | +                    pass
 230 | +                self.wakeup_socket_pair = None
 231 | +            if self.handler_index:

w0xlt commented at 11:34 PM on April 7, 2026:

Can self.handler_index be 0 ?

            if self.handler_index is not None:

optout21 commented at 6:21 AM on April 8, 2026:

Applied.

fanquake renamed this:
~~test: CI fix: Clean shutdown in Socks5Server~~
test: Clean shutdown in Socks5Server
on Apr 8, 2026

optout21 force-pushed on Apr 8, 2026

DrahtBot added the label CI failed on Apr 8, 2026

optout21 marked this as ready for review on Apr 8, 2026

optout21 commented at 6:21 AM on April 8, 2026: contributor

Windows CI fixed (due to select/pipe issue), commits squashed, ready for review. @w0xlt , thanks for the remarks, applied them (prevent reuse of indexes, handlers stored in a sparse array), added you as co-author (also @vasild ).

DrahtBot removed the label CI failed on Apr 8, 2026

in test/functional/test_framework/socks5.py:231 in 14bbcad993

 238 | +            if self.wakeup_socket_pair:
 239 | +                self.wakeup_socket_pair[0].send("CloseWakeup".encode())
 240 | +                logger.debug("Waking up forwarding thread")
 241 | +        except OSError as e:
 242 | +            logger.warning(f"Error waking up forwarding thread: {e}")
 243 | +            pass

vasild commented at 3:44 PM on April 8, 2026:

This shouldn't be called force_close() anymore. Maybe wakeup()?

optout21 commented at 10:11 AM on April 9, 2026:

Renamed. It is called during force close, but unblock or wakeup indeed describes it better what it does.

in test/functional/test_framework/socks5.py:227 in 14bbcad993

 234 | +
 235 | +    def force_close(self):
 236 | +        # Wake up the blocking forwarding select by writing to the wake-up socket
 237 | +        try:
 238 | +            if self.wakeup_socket_pair:
 239 | +                self.wakeup_socket_pair[0].send("CloseWakeup".encode())

vasild commented at 3:49 PM on April 8, 2026:

This works. Just mentioning an alternative - closing the socket for reading wakes up the select() if the socket is in the "read list".

self.wakeup_socket_pair[1].shutdown(socket.SHUT_RD)

can use wakeup_socket_pair[0] as well, but then must pass that one to forward_sockets() instead of [1].

vasild commented at 4:06 PM on April 8, 2026:

wakeup_socket_pair could be changed to None by another thread after the if and before the wakeup_socket_pair[0].send() call. It needs to be either protected by a mutex (better) or must expect and catch "TypeError: 'NoneType' object is not subscriptable".

optout21 commented at 10:13 AM on April 9, 2026:

Thanks, but a plain send is conceptually simpler (sending actual data is not really necessary, though). Leaving as-is.

optout21 commented at 10:15 AM on April 9, 2026:

Good observation. To mitigate it, now self.wakeup_socket_pair is copied to a local variable, tested, and then used.

in test/functional/test_framework/socks5.py:218 in 14bbcad993

 225 | +                try:
 226 | +                    self.wakeup_socket_pair[0].close()
 227 | +                    self.wakeup_socket_pair[1].close()
 228 | +                except OSError:
 229 | +                    pass
 230 | +                self.wakeup_socket_pair = None

vasild commented at 3:57 PM on April 8, 2026:

This first closes the sockets and then sets wakeup_socket_pair[] to None. So there is some period of time where two numbers are in wakeup_socket_pair[] which do not correspond to any sockets (have been closed). Attempting to write to such one would cause "bad file descriptor" exception. Or worse, the same number has been assigned to a newly created file descriptor (somebody else opened a file or a socket). In this case the write would succeed, but it would write to the wrong file descriptor! This could happen if force_close() is called during the period of time when the "stale" numbers are in wakeup_socket_pair[].

To resolve this first assign wakeup_socket_pair = None and then close the sockets.

s0 = self.wakeup_socket_pair[0]
s1 = self.wakeup_socket_pair[1]
self.wakeup_socket_pair = None
try:
    s0.close()
    s1.close()
except OSError:
    pass

optout21 commented at 10:13 AM on April 9, 2026:

Applied.

in test/functional/test_framework/socks5.py:212 in 14bbcad993

 219 |          finally:
 220 |              if not self.serv.keep_alive:
 221 |                  self.conn.close()
 222 |              else:
 223 |                  logger.debug("Keeping client connection alive")
 224 | +            if self.wakeup_socket_pair:

vasild commented at 4:00 PM on April 8, 2026:

This if will always evaluate to True, so no need for it. The only code that sets wakeup_socket_pair to None is just inside the if body below.

optout21 commented at 10:13 AM on April 9, 2026:

Applied.

in test/functional/test_framework/socks5.py:285 in 14bbcad993 outdated

 285 | -                thread = threading.Thread(None, conn.handle)
 286 | -                thread.daemon = True
 287 | +                # Use "daemon" threads, see PR #34863 for more discussion.
 288 | +                thread = threading.Thread(None, conn.handle, daemon=True)
 289 | +                with self._handlers_lock:
 290 | +                    conn.handler_index = len(self._handlers)

vasild commented at 4:11 PM on April 8, 2026:

I agree with #34863#pullrequestreview-4071931157, better not use len(self._handlers).

optout21 commented at 10:21 AM on April 9, 2026:

I have considered these two approaches:

A dictionary, with arbitrary keys, and using numbers as keys. Unneeded entries can be removed. A separate variable is needed to assign keys. (My original implementation just use the size of the dict, which is incorrect, as noted by @w0xlt .)
A (sparse) array, used to append only new entries, and the key is the array index. Unneeded entries are set to None (but the slots stay in the array). In this case it's OK to use the size of the array to get the current index.

I choose 2., as it seems a bit simpler. In this case there is no need for a separate variable, as the array is used in append-only mode. When adding, the addition and obtaining the current size is an atomic step (protected by a lock). In fact, using a separate variable has less guarantee that the indices are within the array.

I kept as it is, only added a clarification that the array is used in append-only mode. Also added an assert, that right after adding to the array the index is within the array.

in test/functional/test_framework/socks5.py:221 in 14bbcad993

 228 | +                except OSError:
 229 | +                    pass
 230 | +                self.wakeup_socket_pair = None
 231 | +            if self.handler_index is not None:
 232 | +                self.serv.remove_handler(self.handler_index)
 233 | +                self.handler_index = None

vasild commented at 4:19 PM on April 8, 2026:

self.handler_index will never be None here, can drop the if. Also can drop the check from inside remove_handler() because this is the only caller.

optout21 commented at 10:22 AM on April 9, 2026:

Applied.

vasild commented at 4:20 PM on April 8, 2026: contributor

Approach ACK 14bbcad993

achow101 removed this from the milestone 31.0 on Apr 8, 2026

achow101 added this to the milestone 31.1 on Apr 8, 2026

optout21 force-pushed on Apr 9, 2026

optout21 commented at 10:23 AM on April 9, 2026: contributor

Applied some minor changes following new review from @vasild -- thanks!

in test/functional/test_framework/socks5.py:53 in 42618ebb8f

  48 | @@ -49,7 +49,7 @@ def sendall(s, data):
  49 |                  raise IOError('send() on socket returned 0')
  50 |              sent += n
  51 |  
  52 | -def forward_sockets(a, b):
  53 | +def forward_sockets(a, b, wakeup_socket, serv):
  54 |      """Forward data received on socket a to socket b and vice versa, until EOF is received on one of the sockets."""

andrewtoth commented at 11:39 PM on April 12, 2026:

docstring is stale

    """Forward data between sockets a and b until EOF, error, or shutdown.

    Monitors wakeup_socket for a shutdown signal and checks serv.is_running()
    to exit gracefully when the server is stopping.
    """

optout21 commented at 8:42 AM on April 13, 2026:

Updated.

andrewtoth commented at 2:19 PM on April 13, 2026:

Looks like the first line of the following comment block was removed though:

# Mark as non-blocking so that we do not end up in a deadlock-like situation

optout21 commented at 3:07 PM on April 13, 2026:

Ops, thanks! Corrected.

in test/functional/test_framework/socks5.py:220 in 42618ebb8f

 227 | +            try:
 228 | +                s0.close()
 229 | +                s1.close()
 230 | +            except OSError:
 231 | +                pass
 232 | +            self.wakeup_socket_pair = None

andrewtoth commented at 11:50 PM on April 12, 2026:

This looks like a leftover before it was moved above the try.

optout21 commented at 8:42 AM on April 13, 2026:

Good catch, fixed.

in test/functional/test_framework/socks5.py:318 in 42618ebb8f

 323 | +        # if there are active handlers, close them
 324 | +        with self._handlers_lock:
 325 | +            items = list(self._handlers)
 326 | +        for i in range(len(items)):
 327 | +            if items[i] is not None:
 328 | +                (thread, conn) = items[i]

andrewtoth commented at 12:00 AM on April 13, 2026:

nit: a little easier to read IMO

        for i, item in enumerate(items):
            if item is None:
                continue
            thread, conn = item

optout21 commented at 8:43 AM on April 13, 2026:

Changed. I also prefer flow control commands and less indentation level.

andrewtoth commented at 2:20 PM on April 13, 2026:

I also prefer flow control commands

I see the enumerate suggestion was not taken. Was that intentional?

optout21 commented at 3:07 PM on April 13, 2026:

Not intentional, overlooked, sorry. I change it.

andrewtoth commented at 6:09 PM on April 13, 2026:

Seems like it still wasn't taken. Not a big deal though.

optout21 commented at 6:05 AM on April 14, 2026:

Pushed now. (I've tried to prepare the comments and the commit, and submit them at once, but somehow the push command was still waiting for an Enter to be pressed :P )

in test/functional/test_framework/socks5.py:78 in 42618ebb8f

andrewtoth commented at 12:03 AM on April 13, 2026:

nit: since wakeup_socket is in the sockets list now, this makes the code more explicit. We can't get here since we'd already return above, but that logic depends on code elsewhere.

            elif s == b:

optout21 commented at 8:53 AM on April 13, 2026:

Good point, changed!

optout21 force-pushed on Apr 13, 2026

optout21 commented at 8:51 AM on April 13, 2026: contributor

Applied nits from @andrewtoth 's review -- thanks!

git diff 42618ebb8f5497e7ded268d7e5d8c4d594e94361..76e432df7df55d497682572832facd3f832910a5

test: Add clean shutdown to Socks5Server

The `Socks5Server` utility handles multiple incoming connections,
which are handled in separate background threads.
The `stop()` method unblocks and waits for the main background thread
cleanly, but it doesn't attempt to wait for any handler threads.
This change stores handler threads and connections, and attempts
to shut them down before `stop()` returns.

Co-authored-by: vasild <vd@FreeBSD.org>
Co-authored-by: w0xlt <94266259+w0xlt@users.noreply.github.com>

6ac49373aa

optout21 commented at 3:09 PM on April 13, 2026: contributor

Fix of the last fix 🥴 : a minor nit (continued), and an accidentally removed comment reverted.

git diff 76e432df7df55d497682572832facd3f832910a5..6ac49373aac1228e427e17a4b7b38fc1ba029208 git diff 42618ebb8f5497e7ded268d7e5d8c4d594e94361..6ac49373aac1228e427e17a4b7b38fc1ba029208

in test/functional/test_framework/socks5.py:220 in 76e432df7d outdated

 227 | +            s0 = self.wakeup_socket_pair[0]
 228 | +            s1 = self.wakeup_socket_pair[1]
 229 | +            self.wakeup_socket_pair = None
 230 | +            try:
 231 | +                s0.close()
 232 | +                s1.close()

andrewtoth commented at 3:30 PM on April 13, 2026:

nit: if s0.close() throws, s1 will never be closed.

in test/functional/test_framework/socks5.py:292 in 76e432df7d outdated

 292 | +                # Use "daemon" threads, see PR #34863 for more discussion.
 293 | +                thread = threading.Thread(None, conn.handle, daemon=True)
 294 | +                with self._handlers_lock:
 295 | +                    conn.handler_index = len(self._handlers)
 296 | +                    self._handlers.append((thread, conn))
 297 | +                    assert(conn.handler_index < len(self._handlers))

andrewtoth commented at 3:36 PM on April 13, 2026:

nit: this assert seems unnecessary. _handlers_lock is held and we just set handler_index before starting the thread.

andrewtoth approved

andrewtoth commented at 3:36 PM on April 13, 2026: contributor

ACK 76e432df7df55d497682572832facd3f832910a5

nits are non-blocking.

DrahtBot requested review from vasild on Apr 13, 2026

DrahtBot requested review from achow101 on Apr 13, 2026

optout21 force-pushed on Apr 14, 2026

in test/functional/test_framework/socks5.py:55 in 6ac49373aa

  48 | @@ -49,19 +49,27 @@ def sendall(s, data):
  49 |                  raise IOError('send() on socket returned 0')
  50 |              sent += n
  51 |  
  52 | -def forward_sockets(a, b):
  53 | -    """Forward data received on socket a to socket b and vice versa, until EOF is received on one of the sockets."""
  54 | +def forward_sockets(a, b, wakeup_socket, serv):
  55 | +    """Forwards data between sockets a and b until EOF, error, or shutdown.
  56 | +
  57 | +    Monitors wakeup_socket for a shutdown signal and checks serv.is_running()

vasild commented at 10:25 AM on April 14, 2026:

s/shutdown signal/incoming data/

vasild approved

vasild commented at 10:33 AM on April 14, 2026: contributor

ACK 6ac49373aac1228e427e17a4b7b38fc1ba029208

<details> <summary>Show Signature</summary>

vasild's public key is on openpgp.org

</details>

DrahtBot requested review from andrewtoth on Apr 14, 2026

andrewtoth commented at 3:32 PM on April 14, 2026: contributor

re-ACK 6ac49373aac1228e427e17a4b7b38fc1ba029208

rkrux commented at 12:07 PM on April 15, 2026: contributor

@rkrux , thanks for the log, and I'd appreciate if you could retest on MacOS.

I don't see those errors anymore on my system.

w0xlt commented at 2:31 PM on April 15, 2026: contributor

ACK 6ac49373aac1228e427e17a4b7b38fc1ba029208

optout21 commented at 9:09 AM on April 19, 2026: contributor

Any chance to consider for merge?

achow101 commented at 9:16 PM on April 20, 2026: member

ACK 6ac49373aac1228e427e17a4b7b38fc1ba029208

achow101 merged this on Apr 20, 2026

achow101 closed this on Apr 20, 2026

fanquake referenced this in commit 39455140b9 on Apr 21, 2026

fanquake commented at 9:13 AM on April 21, 2026: member

Backported to 31.x in #35046.