[krynju]

The minimal #72 fix, re-derived from the design described in the #73 review. #73 stays open as the reference for the larger version; this one is meant to replace it. Rebased on main with #86–#89 merged; one commit.

Bug (#72). SSL_write_ex and SSL_read_ex share ssl.lock, and the write BIO wrote to the socket from inside the call. A write waiting for a peer that had stopped reading held the lock, so every read on the stream waited behind it: a full-duplex exchange, or a read beside a large write, deadlocked.

Fix (~190 lines in src/ssl.jl).

• During one of the package's own SSL calls (incall, set under ssl.lock by @geterror), the write BIO appends what the call produces to a per-stream buffer (BIOStreamData). A raw SSL_* call made on ssl.ssl from outside the package has incall false and writes straight to the socket, as before.
• After the call, outside ssl.lock, a writer (unsafe_write, the handshake) calls sendout!: it takes sendlock, swaps the buffer out under ssl.lock and writes it to the socket. Records leave in the order they were produced, and the writer returns only once its bytes are on the socket, or with the error that kept them from getting there (a failed flag covers bytes another task sent or dropped on its behalf).
• A reader (unsafe_read, eof) calls sendreply!: a reply OpenSSL made goes out from a task that only takes a free sendlock. Whoever holds the lock re-checks the buffer under ssl.lock before releasing it, and the append happens under ssl.lock too, so a reply is never left behind and a read never waits behind a parked writer.
• Writes go to OpenSSL in 64 KiB chunks under a writer lock, so concurrent writes arrive whole.
• close sends what is buffered from a task: the close_notify it makes with shutdown, and whatever a failed call left there, the fatal alert OpenSSL produced while failing, so the peer learns why rather than seeing a clean end of stream (review). The task closes the socket in a finally; should that socket write take longer than CLOSE_NOTIFY_GRACE (5 s, a peer that stopped reading) the task closes the socket outright instead, so neither the socket nor sendlock is held forever. After the shutdown close clears the error queue (Fail the SSL call when the write callback's socket write fails #86's fix; a shutdown mid-handshake still queues an error). With a socket write in flight at the time of the close, no close_notify is made, bytes still buffered are dropped and their writers told, and the socket handle is closed outright at once (jl_forceclose_uv, as Base does for an unconnected socket), which fails the parked write. forceclose! goes by the handle and uv_is_closing, not io.status, since Base marks a socket closing itself on the peer's FIN on Julia 1.6/1.7. On Windows it also calls CancelIoEx on the socket after uv_close: libuv's close there cancels only the writes the calling thread issued (CancelIo, a libuv TODO) and waits for the rest, so a write parked by a task on another thread kept the socket open (seen on CI once the close task was spawned: on Julia 1.12+ the main task is on the interactive thread and Threads.@spawn on another).

Compared with #73 this gives up the handshake timeout, the progress-bounded graceful close behind a parked writer, and the protection for tasks cancelled inside a socket write. Per stream it keeps one empty Vector{UInt8} after a send, not a spare buffer. The readbytes/writebytes fields stay (from #88).

Tests (all pass on Linux, Julia 1.12 and 1.6.7 with OpenSSL 3.5 and Julia 1.10 with OpenSSL 1.1, 1 and 4 threads, repeated runs): ConcurrentReadWrite (the #72 scenario: a read beside a parked 32 MiB write), FullDuplex (8 MiB each way, both ends reading and writing), ConcurrentWriters (now also whole writes), CloseWithParkedWriter (close returns at once, the writer fails), CloseRacingWriter (a write completing as close runs returns success and arrives; the client reads once first so the server's session tickets are not left unread, since on Windows the reset that a close with unread data sends discards what the peer has not read yet), CloseBehindRawWrite and CloseBehindRawWriteNoShutdown (the bounded close behind a raw write, with and without a close_notify to send; written until it parks, since Windows takes a whole 64 MiB write with no reader), FatalAlertReachesPeer (a forged record: the server's read fails and the client's next read gets the alert, not EOF), CloseNotifyEOF (writes before close arrive, then the close_notify), WriteSizes (sizes around a record and a 1 MiB write), ReplyBehindSender (the hand-off when sendlock is held), LegacyAccept (one round, unchanged), RawSSLCalls (raw accept loop under ssl.lock, a raw write landing whole among a parked write's records), PeerResetMidRecord. Confirmed on main's source: FullDuplex fails its 120 s timeout and ConcurrentReadWrite hangs in close waiting for ssl.lock.

Documented limits (not handled, as before this PR for the first): a raw SSL_* call must not overlap the package's own calls on the same stream, or its record overtakes buffered ciphertext on the wire; and write(ssl, …) must not be called with ssl.lock held, since sendout! would then wait on a sendlock holder that needs ssl.lock.

A fourth pass found and I fixed: the close task cancelled the grace timer before Base's socket close, which waits for the socket's pending writes, so a close(ssl, false) (the failed-call path, the finalizer) behind a parked raw write never closed the socket; the timer now covers the socket close too (CloseBehindRawWriteNoShutdown). Also drainlocked! releases sendlock in a finally on its error path, and sendout! names the failing call (connect or write). A third pass found and I fixed: the reply and close tasks were @asynced, which pins the calling task (a reader, a closer) to its thread for good on Julia 1.7+, now Threads.@spawn; and sendreply! read the buffer's emptiness outside ssl.lock, now under it. A second review pass (Opus) found and I fixed: forceclose! skipping a socket Base had already marked closing (Julia 1.6/1.7 on FIN), the close task leaking the socket when the close_notify write failed, the unbounded close_notify drain, and a writer failing although its bytes were delivered when close cut a write that had in fact just completed.

Base internals used: jl_forceclose_uv with iolock_begin/iolock_end and the socket's status, the same as Base's own close; the tests also read uv_stream_get_write_queue_size.

Two points I'd like a second look at, since this is re-derived rather than your prototype: the lock order (sendlock is taken before ssl.lock, never the other way round blocking; the reply task uses trylock), and the inflight bookkeeping that close reads to decide between sending the close_notify and cutting the socket.

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 92.37288% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.07%. Comparing base (10f7b7b) to head (1e3281a).
⚠️ Report is 60 commits behind head on main.

Files with missing lines	Patch %	Lines
src/ssl.jl	92.37%	9 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #91      +/-   ##
==========================================
+ Coverage   77.56%   83.07%   +5.50%     
==========================================
  Files           2        2              
  Lines        1083     1341     +258     
==========================================
+ Hits          840     1114     +274     
+ Misses        243      227      -16     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[quinnj]

[review generated via AI]

Thanks Krystian, this is the shape I was hoping for. #72 is fixed: 8 MiB each way at once hangs on main and finishes in about 0.2 s here. The suite passes on macOS with Julia 1.12 and 1.10, 1 and 4 threads, and with OpenSSL 1.1. The lock order and the reply hand-off look right to me. A few things before merging:

• Fatal alerts no longer reach the peer. When one of the package's SSL calls fails, @geterror calls close(ssl, false), which empties data.buf, where OpenSSL has just put its alert. With a forged record, main's server read fails with "tls alert bad record mac"; on this branch the server sees a clean eof() == true. Could close drop the buffer only when it cuts an in-flight write, and otherwise let the close task send what's buffered, with or without shutdown?
• The Windows failures come from the test. CloseBehindRawWrite waits for writequeuesize(client.io) > 0 after a single 64 MiB raw write, before any package code runs. Windows accepts the whole write with no reader, so the queue never fills. Writing until the writer parks, as park_writer does, should fix it.
• Nit: "as before" in the BIOStreamData docstring describes history; the docstring should only say what the code does now.
• I merged #86 and am merging #88 (both rebased on current main), so please rebase this on main. close will conflict with #86's clear_errors!(); keep it, since a shutdown mid-handshake still leaves an error on the queue.

Once this lands I'll close #73 as superseded.

[quinnj]

[review generated via AI]

Thanks Krystian. This round fixes everything from the last review. The alert from a failed call now reaches the peer. The Windows test writes until the writer parks. The branch is on current main and keeps #86's clear_errors!(). CI is green, and the suite passes here on macOS with Julia 1.12 and 10 threads. Bulk transfer is about 2.5x faster than main: sending 256 MiB in 1 MiB writes over loopback, the median was 307 MiB/s against 120. Small writes and ping-pong showed no slowdown, though this machine is loaded and those numbers are noisy. A 64-byte write allocates about 227 bytes, against 83 on main.

One blocker before merging:

The process can crash at exit. close now starts its send-and-close task with Threads.@spawn, which puts the task on the :default pool. The finalizer calls close(x, false), and Julia runs every finalizer at exit, including those of live objects. So when a stream is still open at exit (an HTTP.jl 1.x pooled connection, for example), a worker thread starts compiling that task while the main thread is already in C exit(), destroying LLVM's globals. The process then segfaults or aborts, with the stack start_task → jl_compile_codeinst_now → LLVM. JuliaServices/Reseau.jl#173 fixed the same crash.

Repro: the script below opens 8 client/server pairs, writes once on each, leaves them open and calls exit(1). I ran it with julia -t 8; a clean exit returns 1. To widen the exit window I preloaded a library whose destructor sleeps 10 s (EXIT_DELAY_MS=10000 DYLD_INSERT_LIBRARIES=libexitdelay.dylib).

runs	this PR	main
10 s exit delay	5 of 24 crashed	0 of 24
no delay, other threads kept busy with yield() loops	1 of 48	0 of 48

Suggested fix: the finalizer frees the SSL object and closes the socket handle itself, without starting a task.

function finalize_stream(ssl::SSLStream)
    Base.@lock ssl.lock begin
        ssl.closed && return
        ssl.closed = true
        free(ssl.ssl)
    end
    forceclose!(ssl.io)
    return
end

With finalizer(finalize_stream, x), the same runs gave 0 of 24 and 0 of 48, and the suite passes. For a deterministic test: on this branch, client.io.status right after finalize(client) is still Base.StatusOpen, because the close waits for the task. With the change it is Base.StatusClosing.

An explicit close just before exit, which is the usual end of a script, hits the same race less often. With the 10 s delay it crashed 1 of 72 runs with Threads.@spawn, and 0 of 96 with @async. Starting close's task with @async, as main does, avoids it. That pins the closing task to its thread, which main already does in close. sendreply! can stay on Threads.@spawn.

Smaller things:

• close lost its docstring. CLOSE_NOTIFY_GRACE and its comment now sit between """Close SSL stream.""" and the method, so ?close shows nothing for SSLStream. Moving the const above the docstring fixes it.
• Windows forceclose!: when no write is pending, libuv's uv__tcp_close (src/win/tcp.c) calls closesocket immediately. The CancelIoEx(sock[], C_NULL) after jl_forceclose_uv can then hit a handle value that another thread has already reused. Calling it just before jl_forceclose_uv, inside the same iolock section, should cancel the same writes while the handle is still valid.
• The spawned tasks rethrow errors other than IOError into a task that nothing waits on. Wrapping them in errormonitor would print those errors.
• If the finalizer changes, update the comment in CloseBehindRawWriteNoShutdown, which says the finalizer uses close(ssl, false).

I'm happy to merge once the exit crash is fixed.

Repro script

using OpenSSL, Sockets, Dates

function selfsigned_server_ctx()
    cert = X509Certificate()
    key = EvpPKey(rsa_generate_key())
    cert.public_key = key
    name = X509Name()
    add_entry(name, "CN", "localhost")
    cert.subject_name = name
    cert.issuer_name = name
    Dates.adjust(cert.time_not_before, Second(0))
    Dates.adjust(cert.time_not_after, Year(1))
    sign_certificate(cert, key)
    ctx = OpenSSL.SSLContext(OpenSSL.TLSServerMethod(), "")
    OpenSSL.ssl_use_certificate(ctx, cert)
    OpenSSL.ssl_use_private_key(ctx, key)
    return ctx
end

function accept_handshake(ssl)
    while true
        try
            return Sockets.accept(ssl)
        catch ex
            ex isa OpenSSL.OpenSSLError || rethrow()
            eof(ssl.io) && throw(EOFError())
        end
    end
end

const SERVER_CTX = selfsigned_server_ctx()
const PORT, SERVER = Sockets.listenany(ip"127.0.0.1", 20000)
const STREAMS = Any[]
for _ in 1:8
    st = @async begin
        ssl = OpenSSL.SSLStream(SERVER_CTX, accept(SERVER))
        accept_handshake(ssl)
        ssl
    end
    client = OpenSSL.SSLStream(OpenSSL.SSLContext(OpenSSL.TLSClientMethod(), ""),
        Sockets.connect(ip"127.0.0.1", PORT))
    Sockets.connect(client; require_ssl_verification=false)
    server = fetch(st)
    write(client, "ping")
    String(read!(server, Vector{UInt8}(undef, 4))) == "ping" || error("bad read")
    push!(STREAMS, client, server)
end
sleep(0.2)
exit(1)

The preload library is a C file containing only __attribute__((destructor)) static void exit_delay(void) { usleep(1000 * atoi(getenv("EXIT_DELAY_MS"))); }, built with cc -dynamiclib.

[krynju]

I can get to this next week. Feel free to commit adjustments and merge

[quinnj-bot]

[comment generated via AI]

Thanks Krystian. As you suggested, I pushed the fixes from the last review (1e3281a) and am merging:

• The finalizer frees the SSL object and closes the socket itself, without starting a task. A stream whose lock is held waits for a later collection. With 8 live stream pairs at exit, a 10 s exit delay and -t 8, crashes went from 4 of 24 runs to 0 of 24. A close just before exit crashed 0 of 48 runs.
• close starts its task with @async again, as main does. Both tasks now have an errormonitor.
• On Windows, forceclose! cancels the socket's I/O before closing it, while the handle is still valid.
• close has its docstring back, and a new test, FinalizerClosesInPlace, covers the finalizer.

The suite passes locally on Julia 1.12 with 4 threads and on Julia 1.10 with 1 thread, and CI is green.