Conversation
…ad pipe A conn.sock client that disconnects while replies are still owed could segfault the king. The read side hitting EOF and each queued write failing all called _conn_moor_bail for the same chan, and the bail path itself tried to write a %bail noun to the broken pipe, which could fail synchronously and re-enter _conn_moor_bail before liv_o was cleared. Every call ran _conn_close_chan, which called uv_close on handles that were already closing and queued a second %done to khan, while the chan was freed from the first close callback. Return early from _conn_close_chan and _conn_moor_bail when the pipe is already closing, clear liv_o before the courtesy %bail write and skip that write entirely on EPIPE/ECONNRESET, and have u3_newt_send drop writes on a closing stream instead of reporting their failure into a teardown already in progress.
8d25707 to
8c38bc5
Compare
|
I'm unable to reproduce the crash on develop. Are you sure the clanker built the binary from develop and not vere-v4.6? Master doesn't yet have c0a35c6. |
|
I told it to test against the latest but had it do it again and improve the example case below: Built from Build provenance. The tree is a clone of urbit/vere built at The printed rev can't be used to tell my two binaries apart. Why it probably didn't reproduce. It needs enough pipelined replies to exceed the socket send buffer, so writes are still queued when the client goes away. Below that threshold the ship never even sees EPIPE. Same pier, same script, only the request count varying, on
So a client that sends one command and closes (the scenario in c0a35c6) is fine on develop. Several hundred pipelined requests whose replies are never read is not. Correction to the issue. The With this branch, the same 400-request run three times in a row: 3 bail lines total, one per run, ship still answering. Reproducer, self-contained, nothing but python3 (the version in the issue omitted the jam/newt helpers, which I suspect is the actual gap). import os, socket, sys, time
def jam(n):
out = pos = 0
def put(v, w):
nonlocal out, pos
out |= (v & ((1 << w) - 1)) << pos; pos += w
def mat(a):
if a == 0:
put(1, 1); return
b = a.bit_length(); c = b.bit_length()
put(1 << c, c + 1)
put(b & ((1 << (c - 1)) - 1), c - 1)
put(a, b)
def enc(x):
if isinstance(x, tuple):
put(1, 2); enc(x[0]); enc(x[1]) # cell
else:
put(0, 1); mat(x) # atom
enc(n)
return out.to_bytes(max(1, (pos + 7) // 8), 'little')
def newt(n):
b = jam(n); return b'\x00' + len(b).to_bytes(4, 'little') + b
def tas(s): return int.from_bytes(s.encode(), 'little')
def path(*segs):
n = 0
for s in reversed(segs): n = (tas(s), n)
return n
def peek(view, desk, *spur):
sp = path(*spur) if spur else (0, 0)
return (tas('peek'), (1, (tas('once'), (tas(view), (tas(desk) if desk else 0, sp)))))
pier, count = sys.argv[1], int(sys.argv[2]) if len(sys.argv) > 2 else 200
sock = os.path.join(pier, '.urb', 'conn.sock')
s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
s.connect(sock)
for i in range(count):
s.sendall(newt((i, peek('e', 'bindings'))))
s.sendall(newt((10000 + i, peek('bx', '', 'debug', 'timers'))))
s.close() # leave every reply owed
print('sent %d requests, closed without reading' % (count * 2))
time.sleep(10)
c = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
c.settimeout(10)
try:
c.connect(sock); c.sendall(newt((1, (tas('peel'), path('live'))))); c.recv(64)
print('ship still answers -> PASS')
except Exception as e:
print('ship does not answer: %r -> FAIL' % (e,)); sys.exit(1)Note the AF_UNIX 108-char path limit if your pier lives somewhere deep. Ship was a fake ship booted from a brass pill; |
2 participants
Description
Resolves #1100.
A
conn.sockclient that disconnects while the king still owes it replies can take the ship down. Channel teardown was not idempotent, and the error path re-entered itself:EPIPE→_newt_write_cb→_conn_moor_bail._conn_moor_bailtries to send a[0 %bail ...]noun to the client that just went away. That write goes to the same broken pipe.u3_newt_sendcallsbal_fdirectly whenuv_writefails synchronously, so_conn_moor_bailre-enters — and it re-enters beforeliv_ois cleared, so it sends again._conn_moor_bailfor the same chan. Every call ran_conn_close_chan, which calledu3_newt_moat_stop→uv_closeon handles that were already closing and queued another%doneto khan, while the chan is freed from the first close callback.Changes:
_conn_close_chan: return early if the chan's pipe is already closing, making teardown idempotent._conn_moor_bail: return early if already closing; clearliv_obefore the courtesy%bailwrite; and skip that write entirely onEPIPE/ECONNRESET, where the peer is provably gone and the write can only fail and re-enter.u3_newt_send: drop the write (freeing the buffer) if the stream is already closing rather than submitting it and reporting the inevitable failure throughbal_finto a teardown already in progress._newt_write_cbalready treats writes on a closing handle as canceled; this makes the submit side agree.Testing
Both binaries built from this tree with zig 0.15.2 (
zig build -Doptimize=ReleaseFast -Dtarget=x86_64-linux-musl), run against the same fake ship pier with the same script: pipeline N peeks over one conn.sock connection, then close the socket without reading any replies.The crash needs enough pipelined replies to exceed the socket send buffer, so writes are still queued when the client goes away. Below that threshold the ship never sees EPIPE at all. On unpatched
dbc562a:conn: moor baillinesA client that sends a single command and closes — the case c0a35c6 addresses — is unaffected on develop. Several hundred pipelined requests whose replies are never read is not.
With this branch, 400 requests three times in a row: 3
conn: moor baillines total, one per run, ship still answering. There is noloom: external faultstack trace on develop; the process simply takes SIGSEGV after the bail storm.A self-contained reproducer (python3 only, no helpers) is in the PR comments.
Related
#490 — same symptom, could not be reproduced at the time. The reproducer in #1100 hits it reliably.