You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This issue was first seen while working on adding coverage for ListenConnections in #310. While reviewing bitcoin/bitcoin#35037, I came across it again while experimenting with socat. It's trivial to hit because of the new -ipcbind settings introduced there.
Because the connection slots are limited to only one, a single connection is enough to saturate it and start queuing new ones.
Open another terminal (Terminal B), try to connect to the listener again using the command from step 3. Since the listener is already saturated, the connection shouldn't be accepted, so no new "socket connected" log line should appear.
At this point, the second connection is queued, so close it in Terminal B.
Next, close the accepted connection in Terminal A so the listener attempts to accept the already-dead socket that is next in the queue, triggering the bug.
This is a Cap'n Proto bug, @ViniciusCestarii found the root cause (see #310 (review)): After accept(), kj always calls setsockopt(TCP_NODELAY) on the connection socket. For Unix sockets, kj handles the usual "not supported" error codes. On macOS, EINVAL is returned when the client has already disconnected, but kj only tolerates that code when built for FreeBSD. The unexpected error then goes uncaught in the accept loop (escapes mp::ListenConnections), causing the listener to stop accepting new connections.
It's fixed on capnproto's v2 branch (capnproto/capnproto@7df5bd078), but no release includes it. I proposed another solution at the libmultiprocess layer, but it unconditionally (and dangerously) continued the accept loop, ignoring errors that may deserve attention.
Until it's released (or backported), a temporary workaround may be to add a patch to Bitcoin Core's depends, as suggested by @ryanofsky in #310 (review). I've already opened a PR to Bitcoin Core: bitcoin/bitcoin#35796.
In macOS, when a queued client disconnects before being accepted,
ListenConnections's accept path throws:This issue was first seen while working on adding coverage for
ListenConnectionsin #310. While reviewing bitcoin/bitcoin#35037, I came across it again while experimenting with socat. It's trivial to hit because of the new-ipcbindsettings introduced there.To reproduce it, here are the steps (macOS):
The ipc: support per-address max-connections options on -ipcbind bitcoin/bitcoin#35037 branch is needed since it has a
max-connectionscli setting available, so pull the changes and compilebitcoin-node.Start
bitcoin-nodein regtest and cap the connection limit at 1In another terminal (let's call it terminal A), connect to the listening address using socat.
The node should log the accepted connection:
Because the connection slots are limited to only one, a single connection is enough to saturate it and start queuing new ones.
Open another terminal (Terminal B), try to connect to the listener again using the command from step 3. Since the listener is already saturated, the connection shouldn't be accepted, so no new "socket connected" log line should appear.
At this point, the second connection is queued, so close it in Terminal B.
Next, close the accepted connection in Terminal A so the listener attempts to accept the already-dead socket that is next in the queue, triggering the bug.
Now the listener has stopped accepting connections
This is a Cap'n Proto bug, @ViniciusCestarii found the root cause (see #310 (review)): After
accept(), kj always callssetsockopt(TCP_NODELAY)on the connection socket. For Unix sockets, kj handles the usual "not supported" error codes. On macOS,EINVALis returned when the client has already disconnected, but kj only tolerates that code when built for FreeBSD. The unexpected error then goes uncaught in the accept loop (escapesmp::ListenConnections), causing the listener to stop accepting new connections.It's fixed on capnproto's v2 branch (capnproto/capnproto@7df5bd078), but no release includes it. I proposed another solution at the libmultiprocess layer, but it unconditionally (and dangerously) continued the accept loop, ignoring errors that may deserve attention.
Until it's released (or backported), a temporary workaround may be to add a patch to Bitcoin Core's depends, as suggested by @ryanofsky in #310 (review). I've already opened a PR to Bitcoin Core: bitcoin/bitcoin#35796.