From 4e2a93771317a99da531af2ea87bec3ec423c3a1 Mon Sep 17 00:00:00 2001 From: krynju Date: Wed, 23 Sep 2026 09:33:43 +0200 Subject: [PATCH 01/25] fix: buffer ciphertext in the write BIO instead of blocking under ssl.lock `SSL_write_ex`, `SSL_connect`, `SSL_accept` and `SSL_shutdown` all run with `ssl.lock` held, and `SSL_read_ex` needs the same lock. The write BIO callback wrote to the socket from inside those calls, so a write waiting for the peer's receive window blocked every read on the same connection, and traffic that saturates both directions at once deadlocked until one side closed. The read callback already avoids this: it returns a retry when the socket has nothing buffered and the waiting happens after the lock is released. The write callback now appends to a buffer owned by `BIOStreamData` and returns, and `drain!` hands that ciphertext to the socket after each SSL call, outside `ssl.lock`. Back pressure is unchanged: the task that called `write` is the one that waits in `drain!`. Ordering is kept by a second lock that only the socket writes take, and `drain!` returns immediately when there is nothing buffered, so a reader never queues behind a blocked writer. The BIO callbacks still accept a plain `IO`, which is what the certificate and key serialization paths pass. Adds a test that starts a server which stops reading, parks an 8 MB write against it, and reads bytes the peer had already sent. Without the fix the read never completes. Co-Authored-By: Claude Opus 5 (1M context) --- src/ssl.jl | 90 ++++++++++++++++++++++++++++++++++++++++++++---- test/runtests.jl | 72 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+), 7 deletions(-) diff --git a/src/ssl.jl b/src/ssl.jl index a469f2d..a80241e 100644 --- a/src/ssl.jl +++ b/src/ssl.jl @@ -32,10 +32,54 @@ end bio_set_read_retry(bio::BIO) = bio_set_flags(bio, BIO_FLAGS_READ | BIO_FLAGS_SHOULD_RETRY) bio_clear_flags(bio::BIO) = bio_set_flags(bio, 0x00) +""" + What the read and write BIOs of an `SSLStream` are given as their data. + +OpenSSL calls the write BIO from inside `SSL_write_ex`, `SSL_connect`, `SSL_accept` +and `SSL_shutdown`, all of which run with `ssl.lock` held, and `SSL_read_ex` needs that +same lock. Writing to the socket in the callback therefore blocks every read on the +connection for as long as the peer's receive window stays full, which deadlocks any +traffic that saturates both directions at once. The callback only buffers, and +`drain!` moves the ciphertext to the socket afterwards, outside `ssl.lock`. +""" +mutable struct BIOStreamData + io::TCPSocket + # ciphertext produced by OpenSSL that has not reached the socket yet + buf::Vector{UInt8} + # held only to append to or swap out `buf`, never across a socket write + buflock::ReentrantLock + # serializes the socket writes so records leave in the order OpenSSL made them + drainlock::ReentrantLock +end + +BIOStreamData(io::TCPSocket) = BIOStreamData(io, UInt8[], ReentrantLock(), ReentrantLock()) + +""" + Writes whatever OpenSSL has produced to the socket. Must be called without + `ssl.lock` held. Returns without waiting for the socket when there is nothing to + write, so a reader never waits behind a writer that is blocked on the peer. +""" +function drain!(data::BIOStreamData) + Base.@lock(data.buflock, isempty(data.buf)) && return nothing + Base.@lock data.drainlock begin + while true + chunk = Base.@lock data.buflock begin + isempty(data.buf) && break + pending = data.buf + data.buf = UInt8[] + pending + end + GC.@preserve chunk unsafe_write(data.io, pointer(chunk), UInt(length(chunk))) + end + end + return nothing +end + function on_bio_stream_read(bio::BIO, out::Ptr{Cchar}, outlen::Cint) try bio_clear_flags(bio) - io = bio_get_data(bio)::TCPSocket + data = bio_get_data(bio) + io = data isa BIOStreamData ? data.io : data::IO n = bytesavailable(io) if n == 0 bio_set_read_retry(bio) @@ -51,8 +95,17 @@ end function on_bio_stream_write(bio::BIO, in::Ptr{Cchar}, inlen::Cint)::Cint try - io = bio_get_data(bio)::TCPSocket - written = unsafe_write(io, in, inlen) + data = bio_get_data(bio) + if data isa BIOStreamData + # buffer only; `drain!` writes to the socket once `ssl.lock` is free + Base.@lock data.buflock begin + n = length(data.buf) + resize!(data.buf, n + inlen) + GC.@preserve data unsafe_copyto!(pointer(data.buf, n + 1), Ptr{UInt8}(in), UInt(inlen)) + end + return inlen + end + written = unsafe_write(data::IO, in, inlen) return Cint(written) catch e # we don't want to throw a Julia exception from a C callback @@ -412,13 +465,16 @@ mutable struct SSLStream <: IO peekbuf::Base.RefValue{UInt8} peekbytes::Base.RefValue{Csize_t} closed::Bool + # buffers the ciphertext the BIO callbacks produce, see `BIOStreamData` + data::BIOStreamData function SSLStream(ssl_context::SSLContext, io::TCPSocket) # Create a read and write BIOs. - bio_read::BIO = BIO(io; finalize=false) - bio_write::BIO = BIO(io; finalize=false) + data = BIOStreamData(io) + bio_read::BIO = BIO(data; finalize=false) + bio_write::BIO = BIO(data; finalize=false) ssl = SSL(ssl_context, bio_read, bio_write) - x = new(ssl, ssl_context, bio_read, bio_write, io, ReentrantLock(), ReentrantLock(), Ref{Csize_t}(0), Ref{Csize_t}(0), Ref{UInt8}(0x00), Ref{Csize_t}(0), false) + x = new(ssl, ssl_context, bio_read, bio_write, io, ReentrantLock(), ReentrantLock(), Ref{Csize_t}(0), Ref{Csize_t}(0), Ref{UInt8}(0x00), Ref{Csize_t}(0), false, data) finalizer(close, x) return x end @@ -429,6 +485,8 @@ SSLStream(tcp::TCPSocket) = SSLStream(SSLContext(OpenSSL.TLSClientMethod()), tcp # backwards compat Base.getproperty(ssl::SSLStream, nm::Symbol) = nm === :bio_read_stream ? ssl : getfield(ssl, nm) +drain!(ssl::SSLStream) = drain!(getfield(ssl, :data)) + Base.isreadable(ssl::SSLStream)::Bool = isopen(ssl) && isreadable(ssl.io) Base.isopen(ssl::SSLStream)::Bool = Base.@lock(ssl.lock, !ssl.closed) Base.iswritable(ssl::SSLStream)::Bool = isopen(ssl) && isopen(ssl.io) @@ -493,6 +551,9 @@ function Base.unsafe_write(ssl::SSLStream, in_buffer::Ptr{UInt8}, in_length::UIn in_length, ssl.writebytes ) + # the write BIO only buffers, so this is where the socket write happens, and + # the caller waits for the peer here rather than under `ssl.lock` + drain!(ssl) if ret == SSL_ERROR_NONE nwritten += ssl.writebytes[] elseif ret == SSL_ERROR_WANT_WRITE @@ -509,6 +570,7 @@ end function Sockets.connect(ssl::SSLStream; require_ssl_verification::Bool=true) while true ret = @geterror ssl :connect ssl_connect(ssl.ssl) + drain!(ssl) if ret == SSL_ERROR_NONE break elseif ret == SSL_ERROR_WANT_READ @@ -574,7 +636,12 @@ function hostname!(ssl::SSLStream, host) end function Sockets.accept(ssl::SSLStream) - ssl_accept(ssl.ssl) + try + ssl_accept(ssl.ssl) + finally + # the server hello the handshake produced is still in the write buffer + drain!(ssl) + end end """ @@ -593,6 +660,7 @@ function Base.unsafe_read(ssl::SSLStream, buf::Ptr{UInt8}, nbytes::UInt) nbytes - nread, readbytes ) + drain!(ssl) if ret == SSL_ERROR_NONE nread += Base.bitcast(Int, readbytes[]) elseif ret == SSL_ERROR_WANT_READ @@ -677,6 +745,7 @@ function Base.eof(ssl::SSLStream)::Bool 1, ssl.peekbytes ) + drain!(ssl) if ret == SSL_ERROR_NONE return false elseif ret == SSL_ERROR_WANT_WRITE @@ -709,6 +778,13 @@ function Base.close(ssl::SSLStream, shutdown::Bool=true) end free(ssl.ssl) end + if shutdown + try + drain!(ssl) + catch err + @debug "SSL close_notify not sent" err + end + end @async try Base.close(ssl.io) catch e diff --git a/test/runtests.jl b/test/runtests.jl index 15479d2..cbdc7c8 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -632,3 +632,75 @@ end @test_throws ErrorException OpenSSL.SSLContext(ssl_method, "does_not_exist") end + +@testset "ConcurrentReadWrite" begin + # A write that is waiting for the peer must not stop reads on the same stream: + # `SSL_write_ex` and `SSL_read_ex` share `ssl.lock`, so the socket write the write + # BIO does has to happen outside it. + 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) + + server_ctx = OpenSSL.SSLContext(OpenSSL.TLSServerMethod(), "") + OpenSSL.ssl_use_certificate(server_ctx, cert) + OpenSSL.ssl_use_private_key(server_ctx, key) + + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + payload = collect(0x01:0x40) + stop = Channel{Nothing}(1) + server_task = @async begin + sock = accept(server) + ssl = OpenSSL.SSLStream(server_ctx, sock) + while true + try + Sockets.accept(ssl) + break + catch + eof(sock) && return + sleep(0.01) + end + end + write(ssl, payload) + # and from here on the server reads nothing, so the client's write parks + take!(stop) + close(ssl) + end + + client = OpenSSL.SSLStream(OpenSSL.SSLContext(OpenSSL.TLSClientMethod(), ""), + Sockets.connect(ip"127.0.0.1", port)) + Sockets.connect(client; require_ssl_verification=false) + + writer = @async try + write(client, zeros(UInt8, 8 * 1024 * 1024)) + catch ex + ex + end + # give the writer time to fill the peer's receive window and park there + sleep(1.0) + @test !istaskdone(writer) + @test !islocked(client.lock) + + buff = Vector{UInt8}(undef, length(payload)) + reader = @async try + read!(client, buff) + catch ex + ex + end + @test timedwait(() -> istaskdone(reader), 30.0) === :ok + @test buff == payload + + # the server has to go away first: the client still has 8 MB parked against a peer + # that is not reading, so its own close cannot finish until that write fails + put!(stop, nothing) + @test timedwait(() -> istaskdone(server_task), 30.0) === :ok + close(client) + close(server) + @test timedwait(() -> istaskdone(writer), 30.0) === :ok +end From 3e437f36f7b6b33724e6db600ce4243c28413dc7 Mon Sep 17 00:00:00 2001 From: krynju Date: Wed, 23 Sep 2026 10:01:42 +0200 Subject: [PATCH 02/25] test: pick the ReadPEMCert certificate by content, not by position `ReadPEMCert` took the second entry of `MozillaCACerts_jll.cacert` and asserted it carries an `OU`. That is a property of Mozilla's root list, not of the parser: on the bundle shipped with Julia 1.13 and nightly the second entry is COMODO ECC Certification Authority, which has no `OU`, so the testset failed on every platform for those versions and, being a top level testset, took the rest of the file with it. The lts jobs, resolving an older bundle, passed. Search for a certificate that has the fields under test instead. The roots in the bundle are self signed, so the issuer checks still hold. Co-Authored-By: Claude Opus 5 (1M context) --- test/runtests.jl | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index cbdc7c8..79ce7ac 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -106,14 +106,25 @@ end start_line = "==========\n" certs_pem = split(file_content, start_line; keepempty=false) - cert = certs_pem[2] - - x509_cert = X509Certificate(cert) + # the bundle is Mozilla's root list and its order changes with it, so pick a + # certificate that carries the fields under test rather than trusting an index + has_ou(cert) = occursin("/OU=", String(cert.subject_name)) + x509_cert = nothing + for pem in certs_pem + occursin("-----BEGIN CERTIFICATE-----", pem) || continue + candidate = X509Certificate(pem) + if has_ou(candidate) + x509_cert = candidate + break + end + end + @test x509_cert !== nothing @test occursin("/C=", String(x509_cert.subject_name)) @test occursin("/OU=", String(x509_cert.subject_name)) @test occursin("/CN=", String(x509_cert.subject_name)) + # the roots in the bundle are self signed, so the issuer carries the same fields @test occursin("/C=", String(x509_cert.issuer_name)) @test occursin("/OU=", String(x509_cert.issuer_name)) @test occursin("/CN=", String(x509_cert.issuer_name)) From 76fdde677f4836365e189e4f139ab6f67da493c1 Mon Sep 17 00:00:00 2001 From: krynju Date: Wed, 23 Sep 2026 10:15:13 +0200 Subject: [PATCH 03/25] test: detect the parked writer instead of assuming 8 MB is enough The new test asserted that a single 8 MB write is still in flight after a second, which holds where the socket buffers are smaller than that but not on Windows, where all four jobs failed on that assertion alone. Write in a loop instead and wait until the byte count stops moving, which is what "the peer's receive window is full" means on any platform. Co-Authored-By: Claude Opus 5 (1M context) --- test/runtests.jl | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index 79ce7ac..2f773b3 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -688,15 +688,27 @@ end Sockets.connect(ip"127.0.0.1", port)) Sockets.connect(client; require_ssl_verification=false) + # keep writing until the peer's receive window is full and the writer parks: how + # much that takes depends on the platform's socket buffers + stop_writing = Threads.Atomic{Bool}(false) + written = Threads.Atomic{Int}(0) + chunk = zeros(UInt8, 1024 * 1024) writer = @async try - write(client, zeros(UInt8, 8 * 1024 * 1024)) + while !stop_writing[] + write(client, chunk) + Threads.atomic_add!(written, length(chunk)) + end catch ex ex end - # give the writer time to fill the peer's receive window and park there - sleep(1.0) - @test !istaskdone(writer) - @test !islocked(client.lock) + parked = timedwait(60.0; pollint=0.5) do + before = written[] + sleep(0.5) + !istaskdone(writer) && written[] == before + end + @test parked === :ok + # the write waits for the socket without holding the lock reads need + @test timedwait(() -> !islocked(client.lock), 5.0) === :ok buff = Vector{UInt8}(undef, length(payload)) reader = @async try @@ -709,6 +721,7 @@ end # the server has to go away first: the client still has 8 MB parked against a peer # that is not reading, so its own close cannot finish until that write fails + stop_writing[] = true put!(stop, nothing) @test timedwait(() -> istaskdone(server_task), 30.0) === :ok close(client) From 8104a5c35a6327b4987dc3c00ff40878be36f323 Mon Sep 17 00:00:00 2001 From: Krystian Gulinski Date: Wed, 23 Sep 2026 11:02:27 +0200 Subject: [PATCH 04/25] fix: close stream on drain failure, chunk SSL_write_ex submissions With the buffering write BIO a failed socket write no longer surfaces as an SSL error, so `@geterror` never closed the stream and a dead connection kept reporting `isopen(ssl) == true`. `drain!(::SSLStream)` now closes the stream (without shutdown) and rethrows, matching the previous behaviour. `close` drains through the `BIOStreamData` directly to avoid recursing into that path. `unsafe_write` passed the whole input to a single `SSL_write_ex`, which made the write BIO buffer a complete ciphertext copy of the payload before anything reached the socket. Submit at most 1 MiB per call and drain between chunks so the buffer stays bounded. The length argument is now `Csize_t` rather than `Cint`, which also removes truncation of writes above 2 GiB. Co-Authored-By: Claude Fable 5.1 --- src/ssl.jl | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/src/ssl.jl b/src/ssl.jl index a80241e..5b09f1d 100644 --- a/src/ssl.jl +++ b/src/ssl.jl @@ -485,7 +485,17 @@ SSLStream(tcp::TCPSocket) = SSLStream(SSLContext(OpenSSL.TLSClientMethod()), tcp # backwards compat Base.getproperty(ssl::SSLStream, nm::Symbol) = nm === :bio_read_stream ? ssl : getfield(ssl, nm) -drain!(ssl::SSLStream) = drain!(getfield(ssl, :data)) +function drain!(ssl::SSLStream) + try + drain!(getfield(ssl, :data)) + catch + # a failed socket write used to surface through the BIO callback as an SSL + # error, which closed the stream; keep that so `isopen` does not report a + # connection whose ciphertext never reached the peer as usable + close(ssl, false) + rethrow() + end +end Base.isreadable(ssl::SSLStream)::Bool = isopen(ssl) && isreadable(ssl.io) Base.isopen(ssl::SSLStream)::Bool = Base.@lock(ssl.lock, !ssl.closed) @@ -539,16 +549,23 @@ macro geterror(ssl, op, expr) end) end +# the write BIO buffers the whole output of one `SSL_write_ex` before `drain!` moves it +# to the socket, so cap how much plaintext goes into a single call to bound that buffer +const SSL_WRITE_CHUNK = UInt(1 << 20) + function Base.unsafe_write(ssl::SSLStream, in_buffer::Ptr{UInt8}, in_length::UInt) nwritten = 0 while nwritten < in_length + # SSL_write_ex writes all or nothing without SSL_MODE_ENABLE_PARTIAL_WRITE, so a + # retry after WANT_READ/WANT_WRITE resubmits the same chunk + chunk = min(in_length - nwritten, SSL_WRITE_CHUNK) ret = @geterror ssl :unsafe_write ccall( (:SSL_write_ex, libssl), Cint, - (SSL, Ptr{Cvoid}, Cint, Ptr{Csize_t}), + (SSL, Ptr{Cvoid}, Csize_t, Ptr{Csize_t}), ssl.ssl, - in_buffer, - in_length, + in_buffer + nwritten, + chunk, ssl.writebytes ) # the write BIO only buffers, so this is where the socket write happens, and @@ -780,7 +797,7 @@ function Base.close(ssl::SSLStream, shutdown::Bool=true) end if shutdown try - drain!(ssl) + drain!(getfield(ssl, :data)) catch err @debug "SSL close_notify not sent" err end From e8916bc3a3b63c2c840975d9be758ecd494458ee Mon Sep 17 00:00:00 2001 From: Krystian Gulinski Date: Wed, 23 Sep 2026 11:36:18 +0200 Subject: [PATCH 05/25] chore: bump version to 1.6.2 Co-Authored-By: Claude Fable 5.1 --- Project.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Project.toml b/Project.toml index e359746..a4706cd 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "OpenSSL" uuid = "4d8831e6-92b7-49fb-bdf8-b643e874388c" -version = "1.6.1" +version = "1.6.2" authors = ["Greg Lapinski ", "Jacob Quinn "] [deps] From eb7e3acab95804274e470428bf8056500e5f7499 Mon Sep 17 00:00:00 2001 From: Krystian Gulinski Date: Mon, 28 Sep 2026 10:26:15 +0200 Subject: [PATCH 06/25] fix: each SSL call owns and orders its own ciphertext The write BIO buffer was shared: whoever reached `drain!` first wrote whatever was in it. Between a writer releasing `ssl.lock` and taking `drainlock`, a concurrent reader's `drain!` could pick up the writer's records and block on the peer inside `unsafe_read` or `eof`, the same stall as #72 through a narrower window. A second writer could also see its records swallowed by the first writer's drain loop and return before they reached the socket, missing the failure if they never did. `@geterror` now takes the ciphertext the SSL call produced while it still holds `ssl.lock`, so `buf` only ever holds the records of the call in progress, and hands it a ticket. `drain!` writes chunks in ticket order under a `Threads.Condition`, so records leave in the order OpenSSL made them and every task waits for its own bytes. `buflock` and `drainlock` are gone: the callback and `take!` both run under `ssl.lock`. Fatal alerts reach the peer again. The error paths in `@geterror` close the stream under the lock, take the alert OpenSSL queued, and send it best effort after releasing the lock before closing the socket. `close` is split into `closelocked!` and `closesocket!` for that. `Sockets.accept` goes through `@geterror` too, so it checks `closed`, holds `ssl.lock` around `SSL_accept`, and waits for the peer on `WANT_READ` like `connect` does instead of throwing `OpenSSLError` for the caller to retry. Adds `ConcurrentWriters`: four tasks writing to one stream at once, the server reads everything back intact. Co-Authored-By: Claude Fable 5.1 --- src/ssl.jl | 228 ++++++++++++++++++++++++++++++++--------------- test/runtests.jl | 64 +++++++++++++ 2 files changed, 220 insertions(+), 72 deletions(-) diff --git a/src/ssl.jl b/src/ssl.jl index 5b09f1d..2212b31 100644 --- a/src/ssl.jl +++ b/src/ssl.jl @@ -39,37 +39,81 @@ OpenSSL calls the write BIO from inside `SSL_write_ex`, `SSL_connect`, `SSL_acce and `SSL_shutdown`, all of which run with `ssl.lock` held, and `SSL_read_ex` needs that same lock. Writing to the socket in the callback therefore blocks every read on the connection for as long as the peer's receive window stays full, which deadlocks any -traffic that saturates both directions at once. The callback only buffers, and -`drain!` moves the ciphertext to the socket afterwards, outside `ssl.lock`. +traffic that saturates both directions at once. The callback only buffers; the task +that made the SSL call takes what it produced with `take!` while it still holds +`ssl.lock`, and moves it to the socket with `drain!` after releasing it. + +Every SSL call runs under `ssl.lock` and takes its own output before releasing it, so +`buf` is empty whenever a call starts and only ever holds the records of the call in +progress: a reader never ends up writing a writer's ciphertext to the socket, and each +writer waits for its own bytes. """ mutable struct BIOStreamData io::TCPSocket - # ciphertext produced by OpenSSL that has not reached the socket yet + # ciphertext produced by the SSL call in progress; only touched under `ssl.lock`, + # both by the write BIO callback (OpenSSL runs it inside the SSL call) and by + # `take!` buf::Vector{UInt8} - # held only to append to or swap out `buf`, never across a socket write - buflock::ReentrantLock - # serializes the socket writes so records leave in the order OpenSSL made them - drainlock::ReentrantLock + # socket writes happen in the order the chunks were taken, so records leave in the + # order OpenSSL made them: `take!` hands out a ticket and `drain!` waits for its + # turn. Guards `nextticket` and `turn`. + cond::Threads.Condition + nextticket::Int + turn::Int end -BIOStreamData(io::TCPSocket) = BIOStreamData(io, UInt8[], ReentrantLock(), ReentrantLock()) +BIOStreamData(io::TCPSocket) = BIOStreamData(io, UInt8[], Threads.Condition(), 0, 0) """ - Writes whatever OpenSSL has produced to the socket. Must be called without - `ssl.lock` held. Returns without waiting for the socket when there is nothing to - write, so a reader never waits behind a writer that is blocked on the peer. + Ciphertext one SSL call produced, and its place in the socket write order. """ -function drain!(data::BIOStreamData) - Base.@lock(data.buflock, isempty(data.buf)) && return nothing - Base.@lock data.drainlock begin - while true - chunk = Base.@lock data.buflock begin - isempty(data.buf) && break - pending = data.buf - data.buf = UInt8[] - pending - end - GC.@preserve chunk unsafe_write(data.io, pointer(chunk), UInt(length(chunk))) +struct PendingWrite + chunk::Vector{UInt8} + ticket::Int +end + +""" + take!(data::BIOStreamData) -> Union{PendingWrite, Nothing} + +Takes the ciphertext the SSL call that just returned left in the write BIO. Must run +under `ssl.lock`, before it is released, so the chunk holds this task's records and +nothing another task produced afterwards. Returns `nothing` when the call produced no +output. +""" +function Base.take!(data::BIOStreamData) + isempty(data.buf) && return nothing + chunk = data.buf + data.buf = UInt8[] + ticket = Base.@lock data.cond begin + t = data.nextticket + data.nextticket += 1 + t + end + return PendingWrite(chunk, ticket) +end + +""" + Writes a chunk `take!` returned to the socket. Must be called without `ssl.lock` + held. Chunks go out in ticket order, so a task whose records came after another + task's waits for that task's socket write, and every task waits until its own bytes + have reached the socket, or sees the error when they do not. Nothing to write costs + nothing, so a reader never waits behind a writer that is blocked on the peer. +""" +drain!(data::BIOStreamData, pending::Nothing) = nothing +function drain!(data::BIOStreamData, pending::PendingWrite) + Base.@lock data.cond begin + while data.turn != pending.ticket + wait(data.cond) + end + end + try + chunk = pending.chunk + GC.@preserve chunk unsafe_write(data.io, pointer(chunk), UInt(length(chunk))) + finally + # always pass the turn on, or one failed write would park every later one + Base.@lock data.cond begin + data.turn += 1 + notify(data.cond) end end return nothing @@ -97,12 +141,11 @@ function on_bio_stream_write(bio::BIO, in::Ptr{Cchar}, inlen::Cint)::Cint try data = bio_get_data(bio) if data isa BIOStreamData - # buffer only; `drain!` writes to the socket once `ssl.lock` is free - Base.@lock data.buflock begin - n = length(data.buf) - resize!(data.buf, n + inlen) - GC.@preserve data unsafe_copyto!(pointer(data.buf, n + 1), Ptr{UInt8}(in), UInt(inlen)) - end + # buffer only; the caller of the SSL call this runs inside takes it with + # `take!` and writes it to the socket with `drain!` once `ssl.lock` is free + n = length(data.buf) + resize!(data.buf, n + inlen) + GC.@preserve data unsafe_copyto!(pointer(data.buf, n + 1), Ptr{UInt8}(in), UInt(inlen)) return inlen end written = unsafe_write(data::IO, in, inlen) @@ -485,9 +528,9 @@ SSLStream(tcp::TCPSocket) = SSLStream(SSLContext(OpenSSL.TLSClientMethod()), tcp # backwards compat Base.getproperty(ssl::SSLStream, nm::Symbol) = nm === :bio_read_stream ? ssl : getfield(ssl, nm) -function drain!(ssl::SSLStream) +function drain!(ssl::SSLStream, pending) try - drain!(getfield(ssl, :data)) + drain!(getfield(ssl, :data), pending) catch # a failed socket write used to surface through the BIO callback as an SSL # error, which closed the stream; keep that so `isopen` does not report a @@ -503,49 +546,58 @@ Base.iswritable(ssl::SSLStream)::Bool = isopen(ssl) && isopen(ssl.io) @noinline throwio(op) = throw(Base.IOError("$op requires ssl to be open", 0)) # this is a macro, but should be a function, but closures are stupid slow -# we use this to standardize the error handling for all of the SSL_*_ex functions +# we use this to standardize the error handling for all of the SSL_*_ex functions: +# make the ccall under `ssl.lock`, check the error queue, take the ciphertext the call +# produced while still holding the lock, and write it to the socket after releasing it macro geterror(ssl, op, expr) esc(quote - # lock our SSLStream while we clear errors - # make a ccall, then check the error queue - Base.@lock ssl.lock begin + local _err = nothing + local _ret = SSL_ERROR_NONE + local _pending = Base.@lock $ssl.lock begin # check that SSL is still open before ccall $ssl.closed && throwio($op) # clear the current error queue before openssl ccall clear_errors!() # do the ccall - _ret = $expr + _r = $expr # we want to return one of our SSL return codes, regardless of error - # SSL_peek_ex, SSL_write_ex, SSL_connect, and SSL_read_ex all return 1 on success - if _ret == 1 - ret = SSL_ERROR_NONE - else - err = get_error($ssl.ssl, _ret) - if err == SSL_ERROR_ZERO_RETURN + # SSL_peek_ex, SSL_write_ex, SSL_connect, SSL_accept and SSL_read_ex all + # return 1 on success + if _r != 1 + _e = get_error($ssl.ssl, _r) + if _e == SSL_ERROR_ZERO_RETURN # the peer sent a close_notify, so no more reading is possible - close($ssl, false) - throw(Base.IOError("unexpected EOF", 0)) - elseif err == SSL_ERROR_NONE - ret = SSL_ERROR_NONE - elseif err == SSL_ERROR_WANT_READ - # we need to read more data from the underlying socket - ret = SSL_ERROR_WANT_READ - elseif err == SSL_ERROR_WANT_WRITE - # we need to write more data to the underlying socket + _err = Base.IOError("unexpected EOF", 0) + elseif _e == SSL_ERROR_NONE || _e == SSL_ERROR_WANT_READ || _e == SSL_ERROR_WANT_WRITE + # WANT_READ: we need to read more data from the underlying socket + # WANT_WRITE: we need to write more data to the underlying socket; # we don't expect to ever see this since we set up our SSL # to do auto TLS (re)negotiation - ret = SSL_ERROR_WANT_WRITE + _ret = _e else # this is usually some other kind of error, like a protocol error # or OS-level IO error, just close the SSL connection and throw # notably, the openssl docs say we should *not* call ssl_disconnect # in this case, hence the `false` arg to close - close($ssl, false) - throw(Base.IOError(OpenSSLError(err).msg, 0)) + _err = Base.IOError(OpenSSLError(_e).msg, 0) end end - ret + if _err === nothing + take!(getfield($ssl, :data)) + else + # close under the lock we already hold; what comes back is the alert + # OpenSSL queued for the peer, sent below once the lock is released + closelocked!($ssl, false) + end end + if _err !== nothing + closesocket!($ssl, _pending) + throw(_err) + end + # the write BIO only buffers, so this is where the socket write happens, and + # the caller waits for the peer here rather than under `ssl.lock` + drain!($ssl, _pending) + _ret end) end @@ -568,9 +620,6 @@ function Base.unsafe_write(ssl::SSLStream, in_buffer::Ptr{UInt8}, in_length::UIn chunk, ssl.writebytes ) - # the write BIO only buffers, so this is where the socket write happens, and - # the caller waits for the peer here rather than under `ssl.lock` - drain!(ssl) if ret == SSL_ERROR_NONE nwritten += ssl.writebytes[] elseif ret == SSL_ERROR_WANT_WRITE @@ -587,7 +636,6 @@ end function Sockets.connect(ssl::SSLStream; require_ssl_verification::Bool=true) while true ret = @geterror ssl :connect ssl_connect(ssl.ssl) - drain!(ssl) if ret == SSL_ERROR_NONE break elseif ret == SSL_ERROR_WANT_READ @@ -653,12 +701,34 @@ function hostname!(ssl::SSLStream, host) end function Sockets.accept(ssl::SSLStream) - try - ssl_accept(ssl.ssl) - finally - # the server hello the handshake produced is still in the write buffer - drain!(ssl) + while true + ret = @geterror ssl :accept ccall( + (:SSL_accept, libssl), + Cint, + (SSL,), + ssl.ssl) + if ret == SSL_ERROR_NONE + break + elseif ret == SSL_ERROR_WANT_READ + # this means accept is waiting for more data from the underlying socket + # so call eof on the socket to wait for more bytes to come in + eof(ssl.io) && throw(EOFError()) + else + throw(Base.IOError(OpenSSLError(ret).msg, 0)) + end + end + + # see `connect` + Base.@lock ssl.lock begin + ssl.closed && throwio(:read_ahead) + ccall( + (:SSL_set_read_ahead, libssl), + Cvoid, + (SSL, Cint), + ssl.ssl, + Cint(1)) end + return end """ @@ -677,7 +747,6 @@ function Base.unsafe_read(ssl::SSLStream, buf::Ptr{UInt8}, nbytes::UInt) nbytes - nread, readbytes ) - drain!(ssl) if ret == SSL_ERROR_NONE nread += Base.bitcast(Int, readbytes[]) elseif ret == SSL_ERROR_WANT_READ @@ -762,7 +831,6 @@ function Base.eof(ssl::SSLStream)::Bool 1, ssl.peekbytes ) - drain!(ssl) if ret == SSL_ERROR_NONE return false elseif ret == SSL_ERROR_WANT_WRITE @@ -783,8 +851,19 @@ end Close SSL stream. """ function Base.close(ssl::SSLStream, shutdown::Bool=true) - Base.@lock ssl.lock begin + pending = Base.@lock ssl.lock begin ssl.closed && return + closelocked!(ssl, shutdown) + end + closesocket!(ssl, pending) +end + +# marks the stream closed and frees the SSL object; must run under `ssl.lock`, which +# the caller keeps holding. Returns what OpenSSL left in the write BIO, the close_notify +# when `shutdown` is set or the fatal alert of the call that failed, for `closesocket!` +# to send once the lock is released. +function closelocked!(ssl::SSLStream, shutdown::Bool) + if !ssl.closed ssl.closed = true if shutdown try @@ -795,18 +874,23 @@ function Base.close(ssl::SSLStream, shutdown::Bool=true) end free(ssl.ssl) end - if shutdown - try - drain!(getfield(ssl, :data)) - catch err - @debug "SSL close_notify not sent" err - end + return take!(getfield(ssl, :data)) +end + +# sends what `closelocked!` returned, best effort, then closes the socket. Must be +# called without `ssl.lock` held: the peer may not be reading. +function closesocket!(ssl::SSLStream, pending) + try + drain!(getfield(ssl, :data), pending) + catch err + @debug "SSL alert not sent" err end @async try Base.close(ssl.io) catch e e isa Base.IOError || rethrow() end + return end """ diff --git a/test/runtests.jl b/test/runtests.jl index 2f773b3..4914ac0 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -728,3 +728,67 @@ end close(server) @test timedwait(() -> istaskdone(writer), 30.0) === :ok end + +@testset "ConcurrentWriters" begin + # Two tasks writing to one stream: each SSL call takes its own ciphertext under + # `ssl.lock` and the socket writes go out in that order, so the records arrive in + # the order OpenSSL made them (out of order would fail the record MAC on the peer) + # and neither writer returns before its own bytes reached the socket. + 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) + + server_ctx = OpenSSL.SSLContext(OpenSSL.TLSServerMethod(), "") + OpenSSL.ssl_use_certificate(server_ctx, cert) + OpenSSL.ssl_use_private_key(server_ctx, key) + + nwriters = 4 + nchunks = 64 + chunklen = 64 * 1024 + total = nwriters * nchunks * chunklen + + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + server_task = @async begin + sock = accept(server) + ssl = OpenSSL.SSLStream(server_ctx, sock) + Sockets.accept(ssl) + received = Vector{UInt8}(undef, total) + read!(ssl, received) + close(ssl) + received + end + + client = OpenSSL.SSLStream(OpenSSL.SSLContext(OpenSSL.TLSClientMethod(), ""), + Sockets.connect(ip"127.0.0.1", port)) + Sockets.connect(client; require_ssl_verification=false) + + writers = map(1:nwriters) do id + @async begin + chunk = fill(UInt8(id), chunklen) + for _ in 1:nchunks + write(client, chunk) + end + end + end + @test timedwait(() -> all(istaskdone, writers), 60.0) === :ok + @test !any(istaskfailed, writers) + @test timedwait(() -> istaskdone(server_task), 60.0) === :ok + @test !istaskfailed(server_task) + if !istaskfailed(server_task) + received = fetch(server_task) + @test length(received) == total + # every writer's bytes all arrived, whatever the interleaving + for id in 1:nwriters + @test count(==(UInt8(id)), received) == nchunks * chunklen + end + end + close(client) + close(server) +end From 367e3fdfa53d70cfa03e9f534515981ff7a6c9a0 Mon Sep 17 00:00:00 2001 From: Krystian Gulinski Date: Mon, 28 Sep 2026 11:08:41 +0200 Subject: [PATCH 07/25] fix: consume the ticket of a writer cancelled while waiting its turn The wait for the turn in `drain!` sat outside the `try/finally` that passes the turn on. A task cancelled while parked there (`schedule(t, ex; error=true)`, an interrupt, a timeout wrapper) never consumed its ticket, so `turn` stopped one short of it forever and every writer queued behind it waited on the condition for good. The wait now records a cancelled ticket as abandoned, or passes the turn straight on when the notification and the cancellation raced and the turn was already ours, and `passturn!` skips abandoned tickets. The cancelled writer's record never reaches the peer, which leaves the TLS stream unusable, so `drain!(::SSLStream)` still closes it; the writers behind it now fail on the closed socket instead of hanging, and `close` returns. Adds `CancelledWriter`: a parked writer, one waiting behind it that gets cancelled, and a third behind that which has to finish. Without the fix the third one never does. Co-Authored-By: Claude Fable 5.1 --- src/ssl.jl | 42 +++++++++++++++++---- test/runtests.jl | 95 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+), 8 deletions(-) diff --git a/src/ssl.jl b/src/ssl.jl index 2212b31..fd1cf4a 100644 --- a/src/ssl.jl +++ b/src/ssl.jl @@ -56,13 +56,16 @@ mutable struct BIOStreamData buf::Vector{UInt8} # socket writes happen in the order the chunks were taken, so records leave in the # order OpenSSL made them: `take!` hands out a ticket and `drain!` waits for its - # turn. Guards `nextticket` and `turn`. + # turn. Guards `nextticket`, `turn` and `abandoned`. cond::Threads.Condition nextticket::Int turn::Int + # tickets whose task was cancelled while waiting for its turn; passed over when + # the turn reaches them, so no ticket goes unconsumed and parks every later one + abandoned::Set{Int} end -BIOStreamData(io::TCPSocket) = BIOStreamData(io, UInt8[], Threads.Condition(), 0, 0) +BIOStreamData(io::TCPSocket) = BIOStreamData(io, UInt8[], Threads.Condition(), 0, 0, Set{Int}()) """ Ciphertext one SSL call produced, and its place in the socket write order. @@ -102,8 +105,23 @@ end drain!(data::BIOStreamData, pending::Nothing) = nothing function drain!(data::BIOStreamData, pending::PendingWrite) Base.@lock data.cond begin - while data.turn != pending.ticket - wait(data.cond) + try + while data.turn != pending.ticket + wait(data.cond) + end + catch + # cancelled while waiting (`schedule(task, ex; error=true)`, an interrupt): + # the ticket still has to be consumed, or the turn never gets past it and + # every later writer parks on it. It may already be ours if the notification + # and the cancellation raced. The record this chunk holds never reaches the + # peer, which makes the connection unusable; `drain!(::SSLStream)` closes it, + # so the writers still waiting fail on the socket instead of hanging. + if data.turn == pending.ticket + passturn!(data) + else + push!(data.abandoned, pending.ticket) + end + rethrow() end end try @@ -111,11 +129,19 @@ function drain!(data::BIOStreamData, pending::PendingWrite) GC.@preserve chunk unsafe_write(data.io, pointer(chunk), UInt(length(chunk))) finally # always pass the turn on, or one failed write would park every later one - Base.@lock data.cond begin - data.turn += 1 - notify(data.cond) - end + Base.@lock data.cond passturn!(data) + end + return nothing +end + +# hands the turn to the next ticket that still has a task waiting for it; under `cond` +function passturn!(data::BIOStreamData) + data.turn += 1 + while data.turn in data.abandoned + delete!(data.abandoned, data.turn) + data.turn += 1 end + notify(data.cond) return nothing end diff --git a/test/runtests.jl b/test/runtests.jl index 4914ac0..08ea57f 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -792,3 +792,98 @@ end close(client) close(server) end + +@testset "CancelledWriter" begin + # A writer cancelled while it waits for its turn on the socket must not leave a hole + # in the write order that parks every later writer and `close` forever. Its record + # never reaches the peer, so the stream is closed and later writes fail right away. + 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) + + server_ctx = OpenSSL.SSLContext(OpenSSL.TLSServerMethod(), "") + OpenSSL.ssl_use_certificate(server_ctx, cert) + OpenSSL.ssl_use_private_key(server_ctx, key) + + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + start_reading = Channel{Nothing}(1) + server_task = @async begin + sock = accept(server) + ssl = OpenSSL.SSLStream(server_ctx, sock) + Sockets.accept(ssl) + # read nothing until told to, so the client's writer parks on the socket + take!(start_reading) + nread = 0 + try + while !eof(ssl) + nread += length(readavailable(ssl)) + end + catch ex + # the client's close_notify surfaces as an IOError from the peek in `eof` + ex isa Base.IOError || rethrow() + end + close(ssl) + nread + end + + client = OpenSSL.SSLStream(OpenSSL.SSLContext(OpenSSL.TLSClientMethod(), ""), + Sockets.connect(ip"127.0.0.1", port)) + Sockets.connect(client; require_ssl_verification=false) + + # fill the peer's receive window so the first writer parks on the socket write + stop_writing = Threads.Atomic{Bool}(false) + written = Threads.Atomic{Int}(0) + chunk = zeros(UInt8, 1024 * 1024) + parked_writer = @async try + while !stop_writing[] + write(client, chunk) + Threads.atomic_add!(written, length(chunk)) + end + catch ex + ex + end + parked = timedwait(60.0; pollint=0.5) do + before = written[] + sleep(0.5) + !istaskdone(parked_writer) && written[] == before + end + @test parked === :ok + + # a second writer queues behind it, waiting for its turn, and a third behind that + cancelled_writer = @async write(client, UInt8[1]) + third_writer = @async try + write(client, UInt8[2]) + catch ex + ex + end + @test timedwait(() -> istaskstarted(third_writer), 5.0) === :ok + yield() + @test !istaskdone(cancelled_writer) + @test !istaskdone(third_writer) + # the second one gets cancelled while it waits for its turn + schedule(cancelled_writer, InterruptException(); error=true) + @test timedwait(() -> istaskdone(cancelled_writer), 5.0) === :ok + @test istaskfailed(cancelled_writer) + + # the cancelled writer's record is lost, so the stream is unusable and closed + @test !isopen(client) + stop_writing[] = true + put!(start_reading, nothing) + @test timedwait(() -> istaskdone(parked_writer), 30.0) === :ok + # the writer queued behind the cancelled one fails instead of waiting for a turn + # that never comes + @test timedwait(() -> istaskdone(third_writer), 30.0) === :ok + @test istaskdone(third_writer) && fetch(third_writer) isa Base.IOError + # and neither does close + closer = @async close(client) + @test timedwait(() -> istaskdone(closer), 30.0) === :ok + @test timedwait(() -> istaskdone(server_task), 30.0) === :ok + close(server) +end From 7fc3e8904addfc2aa9b371c7135c5c7d2a1b5105 Mon Sep 17 00:00:00 2001 From: Krystian Gulinski Date: Mon, 28 Sep 2026 11:31:59 +0200 Subject: [PATCH 08/25] fix: eof spun forever on a record truncated by the peer closing When the peer closes mid-record, `SSL_peek_ex` keeps returning WANT_READ, `haspending` stays true for the partial record, and `eof(ssl.io)` returns true at once, so `eof` looped without ever yielding, starving every other task on the thread. Return true there: the bytes that would complete the record are never coming. `CancelledWriter` hit this on macOS, where a client that closes with unread session tickets still gets a FIN through rather than a RST, so the server saw the truncated tail of the parked write followed by EOF. On Linux the RST turned it into an error instead. Adds `TruncatedRecordEOF`, which sends a partial record header and closes the socket cleanly. Co-Authored-By: Claude Fable 5.1 --- src/ssl.jl | 8 ++++++-- test/runtests.jl | 49 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/src/ssl.jl b/src/ssl.jl index fd1cf4a..16ccfb4 100644 --- a/src/ssl.jl +++ b/src/ssl.jl @@ -864,8 +864,12 @@ function Base.eof(ssl::SSLStream)::Bool elseif ret == SSL_ERROR_WANT_READ # if we get WANT_READ back, that means there were pending bytes # to be processed, but not a full record, so we need to wait - # for additional bytes to come in before we can process - eof(ssl.io) + # for additional bytes to come in before we can process. If the + # socket is at EOF they never will: the peer went away mid-record, + # so this is the end of the stream. Looping instead would spin, + # `haspending` stays true for the partial record and `eof(ssl.io)` + # returns at once, without ever yielding. + eof(ssl.io) && return true end end end diff --git a/test/runtests.jl b/test/runtests.jl index 08ea57f..6b721e4 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -887,3 +887,52 @@ end @test timedwait(() -> istaskdone(server_task), 30.0) === :ok close(server) end + +@testset "TruncatedRecordEOF" begin + # A peer that goes away in the middle of a record leaves a partial record buffered: + # `haspending` stays true, `SSL_peek_ex` keeps asking for more bytes, and the socket + # is at EOF. `eof` has to report the end of the stream rather than loop on that. + 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) + + server_ctx = OpenSSL.SSLContext(OpenSSL.TLSServerMethod(), "") + OpenSSL.ssl_use_certificate(server_ctx, cert) + OpenSSL.ssl_use_private_key(server_ctx, key) + + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + handshake_done = Channel{Nothing}(1) + server_task = @async begin + sock = accept(server) + ssl = OpenSSL.SSLStream(server_ctx, sock) + Sockets.accept(ssl) + put!(handshake_done, nothing) + eof(ssl) + end + + client = OpenSSL.SSLStream(OpenSSL.SSLContext(OpenSSL.TLSClientMethod(), ""), + Sockets.connect(ip"127.0.0.1", port)) + Sockets.connect(client; require_ssl_verification=false) + take!(handshake_done) + # take whatever the server sent after the handshake (session tickets) off the + # socket, so that closing it sends a FIN rather than a RST + sleep(0.2) + readavailable(client.io) + # a record header announcing 100 bytes, followed by only 3 of them, then close + write(client.io, UInt8[0x17, 0x03, 0x03, 0x00, 0x64, 0xaa, 0xbb, 0xcc]) + close(client.io) + + # the spin never yields, so on one thread a hang here would starve this task too; + # a task that does not finish is reported by the timeout, a spinning one by CI + @test timedwait(() -> istaskdone(server_task), 30.0) === :ok + @test istaskdone(server_task) && fetch(server_task) === true + close(client) + close(server) +end From ce8741a2d2d769a3c67845151bd53f62c3722f5f Mon Sep 17 00:00:00 2001 From: Krystian Gulinski Date: Mon, 28 Sep 2026 11:54:19 +0200 Subject: [PATCH 09/25] test: report where the CancelledWriter server is stuck when it does not finish macOS CI fails the server_task timeout in this testset and nothing else, and it does not reproduce on Linux or Windows. Record the server's stage and byte count and print them, with the client socket state and the writer results, when the wait times out. Co-Authored-By: Claude Fable 5.1 --- test/runtests.jl | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index 6b721e4..9fdaee9 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -814,23 +814,31 @@ end port, server = Sockets.listenany(ip"127.0.0.1", 20000) start_reading = Channel{Nothing}(1) + # where the server is, for the report if it does not finish + stage = Ref(:accept) + nread = Threads.Atomic{Int}(0) server_task = @async begin sock = accept(server) ssl = OpenSSL.SSLStream(server_ctx, sock) Sockets.accept(ssl) # read nothing until told to, so the client's writer parks on the socket + stage[] = :waiting take!(start_reading) - nread = 0 + stage[] = :reading try while !eof(ssl) - nread += length(readavailable(ssl)) + Threads.atomic_add!(nread, length(readavailable(ssl))) end + stage[] = :eof catch ex # the client's close_notify surfaces as an IOError from the peek in `eof` ex isa Base.IOError || rethrow() + stage[] = :error end + stage[] = :closing close(ssl) - nread + stage[] = :done + nread[] end client = OpenSSL.SSLStream(OpenSSL.SSLContext(OpenSSL.TLSClientMethod(), ""), @@ -884,7 +892,11 @@ end # and neither does close closer = @async close(client) @test timedwait(() -> istaskdone(closer), 30.0) === :ok - @test timedwait(() -> istaskdone(server_task), 30.0) === :ok + server_done = timedwait(() -> istaskdone(server_task), 30.0) + if server_done !== :ok + @info "CancelledWriter: server did not finish" stage=stage[] nread=nread[] written=written[] client_io_status=client.io.status client_io_open=isopen(client.io) parked_writer=fetch(parked_writer) cancelled_writer=cancelled_writer.result third_writer=fetch(third_writer) + end + @test server_done === :ok close(server) end From 3c8fccc3cdd8848fc6183692bd30727b386cd725 Mon Sep 17 00:00:00 2001 From: Krystian Gulinski Date: Mon, 28 Sep 2026 12:16:52 +0200 Subject: [PATCH 10/25] fix: give each SSL call its own byte count `readbytes`, `writebytes` and `peekbytes` were `Ref`s on the stream, shared by every task using it, and read back after `@geterror` released `ssl.lock`. Since the socket write now happens in that gap, another task's `SSL_write_ex` could overwrite the count first: a writer parked on the socket next to a task writing one byte would read back 1, think its chunk was not written, and submit almost all of it a second time, duplicating plaintext on the wire. Reads and peeks had the same hole. The counts are per call now. `ConcurrentWriters` gives every writer a different chunk size, which turns the stale count into a byte count mismatch on the server; with the shared `Ref`s it fails. Co-Authored-By: Claude Fable 5.1 --- src/ssl.jl | 24 ++++++++++++++---------- test/runtests.jl | 10 ++++++---- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/ssl.jl b/src/ssl.jl index 16ccfb4..da85025 100644 --- a/src/ssl.jl +++ b/src/ssl.jl @@ -529,10 +529,6 @@ mutable struct SSLStream <: IO # call `read` or `write` at the same time as per the thread in # https://mailing.openssl.users.narkive.com/HeNGlNAJ/openssl-and-multithreaded-programs lock::ReentrantLock - readbytes::Base.RefValue{Csize_t} - writebytes::Base.RefValue{Csize_t} - peekbuf::Base.RefValue{UInt8} - peekbytes::Base.RefValue{Csize_t} closed::Bool # buffers the ciphertext the BIO callbacks produce, see `BIOStreamData` data::BIOStreamData @@ -543,7 +539,7 @@ mutable struct SSLStream <: IO bio_read::BIO = BIO(data; finalize=false) bio_write::BIO = BIO(data; finalize=false) ssl = SSL(ssl_context, bio_read, bio_write) - x = new(ssl, ssl_context, bio_read, bio_write, io, ReentrantLock(), ReentrantLock(), Ref{Csize_t}(0), Ref{Csize_t}(0), Ref{UInt8}(0x00), Ref{Csize_t}(0), false, data) + x = new(ssl, ssl_context, bio_read, bio_write, io, ReentrantLock(), ReentrantLock(), false, data) finalizer(close, x) return x end @@ -633,6 +629,10 @@ const SSL_WRITE_CHUNK = UInt(1 << 20) function Base.unsafe_write(ssl::SSLStream, in_buffer::Ptr{UInt8}, in_length::UInt) nwritten = 0 + # per call, not per stream: `@geterror` writes to the socket after releasing + # `ssl.lock`, and another task's `SSL_write_ex` would overwrite a shared count + # before it is read back below + writebytes = Ref{Csize_t}(0) while nwritten < in_length # SSL_write_ex writes all or nothing without SSL_MODE_ENABLE_PARTIAL_WRITE, so a # retry after WANT_READ/WANT_WRITE resubmits the same chunk @@ -644,10 +644,10 @@ function Base.unsafe_write(ssl::SSLStream, in_buffer::Ptr{UInt8}, in_length::UIn ssl.ssl, in_buffer + nwritten, chunk, - ssl.writebytes + writebytes ) if ret == SSL_ERROR_NONE - nwritten += ssl.writebytes[] + nwritten += writebytes[] elseif ret == SSL_ERROR_WANT_WRITE flush(ssl.io) elseif ret == SSL_ERROR_WANT_READ @@ -762,7 +762,8 @@ end """ function Base.unsafe_read(ssl::SSLStream, buf::Ptr{UInt8}, nbytes::UInt) nread = 0 - readbytes = ssl.readbytes + # per call, see `unsafe_write` + readbytes = Ref{Csize_t}(0) while nread < nbytes ret = @geterror ssl :unsafe_read ccall( (:SSL_read_ex, libssl), @@ -821,6 +822,9 @@ end function Base.eof(ssl::SSLStream)::Bool bytesavailable(ssl) > 0 && return false + # per call, see `unsafe_write` + peekbuf = Ref{UInt8}(0x00) + peekbytes = Ref{Csize_t}(0) while isopen(ssl) # note that care needs to be taken here to avoid a potential bad # race condition; for SSLStream, we have to manage the state of @@ -853,9 +857,9 @@ function Base.eof(ssl::SSLStream)::Bool Cint, (SSL, Ptr{UInt8}, Cint, Ptr{Csize_t}), ssl.ssl, - ssl.peekbuf, + peekbuf, 1, - ssl.peekbytes + peekbytes ) if ret == SSL_ERROR_NONE return false diff --git a/test/runtests.jl b/test/runtests.jl index 9fdaee9..68a6dd9 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -749,10 +749,12 @@ end OpenSSL.ssl_use_certificate(server_ctx, cert) OpenSSL.ssl_use_private_key(server_ctx, key) + # different sizes per writer: the byte count `SSL_write_ex` reports has to be the + # one of this task's call, not of whichever task ran last on the stream nwriters = 4 nchunks = 64 - chunklen = 64 * 1024 - total = nwriters * nchunks * chunklen + chunklen(id) = id * 32 * 1024 + total = sum(nchunks * chunklen(id) for id in 1:nwriters) port, server = Sockets.listenany(ip"127.0.0.1", 20000) server_task = @async begin @@ -771,7 +773,7 @@ end writers = map(1:nwriters) do id @async begin - chunk = fill(UInt8(id), chunklen) + chunk = fill(UInt8(id), chunklen(id)) for _ in 1:nchunks write(client, chunk) end @@ -786,7 +788,7 @@ end @test length(received) == total # every writer's bytes all arrived, whatever the interleaving for id in 1:nwriters - @test count(==(UInt8(id)), received) == nchunks * chunklen + @test count(==(UInt8(id)), received) == nchunks * chunklen(id) end end close(client) From 7f4560014c324c358ffb4bced44004279aff474b Mon Sep 17 00:00:00 2001 From: Krystian Gulinski Date: Mon, 28 Sep 2026 12:16:52 +0200 Subject: [PATCH 11/25] test: have the server close first in CancelledWriter The server read the client's parked data until EOF, which depends on how the platform tears down a socket closed with unread session tickets on one side and an unsent tail on the other. On the macOS runners the server never saw a FIN or a RST and waited past the timeout, on Linux and Windows it did not. What the test is about, the writer queued behind the cancelled one failing instead of hanging and `close` returning, does not need the data read: the server now goes away first, as in `ConcurrentReadWrite`, which fails the parked write on every platform. Drops the stage report added to find this. Co-Authored-By: Claude Fable 5.1 --- test/runtests.jl | 40 ++++++++++------------------------------ 1 file changed, 10 insertions(+), 30 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index 68a6dd9..d68ebd2 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -815,32 +815,16 @@ end OpenSSL.ssl_use_private_key(server_ctx, key) port, server = Sockets.listenany(ip"127.0.0.1", 20000) - start_reading = Channel{Nothing}(1) - # where the server is, for the report if it does not finish - stage = Ref(:accept) - nread = Threads.Atomic{Int}(0) + stop = Channel{Nothing}(1) server_task = @async begin sock = accept(server) ssl = OpenSSL.SSLStream(server_ctx, sock) Sockets.accept(ssl) - # read nothing until told to, so the client's writer parks on the socket - stage[] = :waiting - take!(start_reading) - stage[] = :reading - try - while !eof(ssl) - Threads.atomic_add!(nread, length(readavailable(ssl))) - end - stage[] = :eof - catch ex - # the client's close_notify surfaces as an IOError from the peek in `eof` - ex isa Base.IOError || rethrow() - stage[] = :error - end - stage[] = :closing + # the server reads nothing, so the client's writer parks on the socket, and it + # goes away first: a client socket closed with data still queued both ways is + # torn down differently from one platform to the next + take!(stop) close(ssl) - stage[] = :done - nread[] end client = OpenSSL.SSLStream(OpenSSL.SSLContext(OpenSSL.TLSClientMethod(), ""), @@ -881,24 +865,20 @@ end schedule(cancelled_writer, InterruptException(); error=true) @test timedwait(() -> istaskdone(cancelled_writer), 5.0) === :ok @test istaskfailed(cancelled_writer) - # the cancelled writer's record is lost, so the stream is unusable and closed @test !isopen(client) + + # the server goes away, which fails the parked write; the writer queued behind the + # cancelled one then fails too, instead of waiting for a turn that never comes stop_writing[] = true - put!(start_reading, nothing) + put!(stop, nothing) + @test timedwait(() -> istaskdone(server_task), 30.0) === :ok @test timedwait(() -> istaskdone(parked_writer), 30.0) === :ok - # the writer queued behind the cancelled one fails instead of waiting for a turn - # that never comes @test timedwait(() -> istaskdone(third_writer), 30.0) === :ok @test istaskdone(third_writer) && fetch(third_writer) isa Base.IOError # and neither does close closer = @async close(client) @test timedwait(() -> istaskdone(closer), 30.0) === :ok - server_done = timedwait(() -> istaskdone(server_task), 30.0) - if server_done !== :ok - @info "CancelledWriter: server did not finish" stage=stage[] nread=nread[] written=written[] client_io_status=client.io.status client_io_open=isopen(client.io) parked_writer=fetch(parked_writer) cancelled_writer=cancelled_writer.result third_writer=fetch(third_writer) - end - @test server_done === :ok close(server) end From e1efe0d78f868e710e59a599ef5907d1532050d4 Mon Sep 17 00:00:00 2001 From: Krystian Gulinski Date: Mon, 28 Sep 2026 12:57:18 +0200 Subject: [PATCH 12/25] fix: send peek output outside eoflock, refuse to write to a closed stream `eof` ran `SSL_peek_ex` through `@geterror`, which drained what the peek produced (a KeyUpdate reply, or the alert for a record that failed) while `eoflock` was still held. With a writer parked on a peer that is not reading, that reader queued behind it under `eoflock`, and every other reader queued behind that: the #72 shape through a rare path. `@geterror` is now `@sslcall`, the part under `ssl.lock` that evaluates to `(ret, pending, err)`, plus `finish_sslcall!`, which sends or throws. `eof` releases `eoflock` before calling the second half and goes round the loop again; the WANT_READ wait stays under `eoflock`, since the race that lock guards against is about that wait. `drain!` takes the owning stream and, once it has the turn, refuses to write when the stream was closed meanwhile. Records before this one were lost, so the write only ever returned success for data the peer would reject; now it is an `IOError` at once. `closesocket!` rethrows anything that is not an `IOError` or `EOFError` from sending the alert, so an interrupt or a cancellation is not swallowed at `@debug`, and closes the socket in `finally` either way. `CancelledWriter` waits until both queued writers are in the condition's wait queue before cancelling one; delivering an exception to a task that is still running on another thread is not allowed. Co-Authored-By: Claude Fable 5.1 --- src/ssl.jl | 101 +++++++++++++++++++++++++++++++---------------- test/runtests.jl | 8 +++- 2 files changed, 73 insertions(+), 36 deletions(-) diff --git a/src/ssl.jl b/src/ssl.jl index da85025..1677706 100644 --- a/src/ssl.jl +++ b/src/ssl.jl @@ -101,9 +101,14 @@ end task's waits for that task's socket write, and every task waits until its own bytes have reached the socket, or sees the error when they do not. Nothing to write costs nothing, so a reader never waits behind a writer that is blocked on the peer. + +`owner`, when given, is the `SSLStream` the chunk belongs to: if it was closed while +this task waited for its turn, the chunk is not written and an `IOError` is thrown +instead. Records before it were lost, so sending it would only have the peer reject +it. The close paths pass no owner: the stream is closed by then by design. """ -drain!(data::BIOStreamData, pending::Nothing) = nothing -function drain!(data::BIOStreamData, pending::PendingWrite) +drain!(data::BIOStreamData, pending::Nothing, owner=nothing) = nothing +function drain!(data::BIOStreamData, pending::PendingWrite, owner=nothing) Base.@lock data.cond begin try while data.turn != pending.ticket @@ -125,6 +130,7 @@ function drain!(data::BIOStreamData, pending::PendingWrite) end end try + owner === nothing || isopen(owner) || throw(Base.IOError("ssl is closed", 0)) chunk = pending.chunk GC.@preserve chunk unsafe_write(data.io, pointer(chunk), UInt(length(chunk))) finally @@ -552,7 +558,7 @@ Base.getproperty(ssl::SSLStream, nm::Symbol) = nm === :bio_read_stream ? ssl : g function drain!(ssl::SSLStream, pending) try - drain!(getfield(ssl, :data), pending) + drain!(getfield(ssl, :data), pending, ssl) catch # a failed socket write used to surface through the BIO callback as an SSL # error, which closed the stream; keep that so `isopen` does not report a @@ -569,9 +575,11 @@ Base.iswritable(ssl::SSLStream)::Bool = isopen(ssl) && isopen(ssl.io) # this is a macro, but should be a function, but closures are stupid slow # we use this to standardize the error handling for all of the SSL_*_ex functions: -# make the ccall under `ssl.lock`, check the error queue, take the ciphertext the call -# produced while still holding the lock, and write it to the socket after releasing it -macro geterror(ssl, op, expr) +# make the ccall under `ssl.lock`, check the error queue, and take the ciphertext the +# call produced while still holding the lock. Evaluates to `(ret, pending, err)`, for +# `finish_sslcall!` once every lock that must not be held across the socket write is +# released; `@geterror` is the two together. +macro sslcall(ssl, op, expr) esc(quote local _err = nothing local _ret = SSL_ERROR_NONE @@ -608,21 +616,32 @@ macro geterror(ssl, op, expr) take!(getfield($ssl, :data)) else # close under the lock we already hold; what comes back is the alert - # OpenSSL queued for the peer, sent below once the lock is released + # OpenSSL queued for the peer, sent by `finish_sslcall!` once the lock + # is released closelocked!($ssl, false) end end - if _err !== nothing - closesocket!($ssl, _pending) - throw(_err) - end - # the write BIO only buffers, so this is where the socket write happens, and - # the caller waits for the peer here rather than under `ssl.lock` - drain!($ssl, _pending) - _ret + (_ret, _pending, _err) end) end +# the second half of an SSL call: send the alert and throw when it failed, otherwise +# write what it produced to the socket and hand back its return code. The write BIO +# only buffers, so this is where the socket write happens, and the caller waits for the +# peer here rather than under `ssl.lock`. +function finish_sslcall!(ssl::SSLStream, ret::SSLErrorCode, pending, err) + if err !== nothing + closesocket!(ssl, pending) + throw(err) + end + drain!(ssl, pending) + return ret +end + +macro geterror(ssl, op, expr) + esc(:(finish_sslcall!($ssl, (@sslcall $ssl $op $expr)...))) +end + # the write BIO buffers the whole output of one `SSL_write_ex` before `drain!` moves it # to the socket, so cap how much plaintext goes into a single call to bound that buffer const SSL_WRITE_CHUNK = UInt(1 << 20) @@ -852,7 +871,7 @@ function Base.eof(ssl::SSLStream)::Bool # at this point, we know there are at least unprocessed bytes # buffered, so we call SSL_peek to get the next record processed, # which still might not result in bytesavailable > 0 - ret = @geterror ssl :peek ccall( + ret, pending, err = @sslcall ssl :peek ccall( (:SSL_peek_ex, libssl), Cint, (SSL, Ptr{UInt8}, Cint, Ptr{Csize_t}), @@ -861,21 +880,31 @@ function Base.eof(ssl::SSLStream)::Bool 1, peekbytes ) - if ret == SSL_ERROR_NONE - return false - elseif ret == SSL_ERROR_WANT_WRITE - flush(ssl.io) - elseif ret == SSL_ERROR_WANT_READ - # if we get WANT_READ back, that means there were pending bytes - # to be processed, but not a full record, so we need to wait - # for additional bytes to come in before we can process. If the - # socket is at EOF they never will: the peer went away mid-record, - # so this is the end of the stream. Looping instead would spin, - # `haspending` stays true for the partial record and `eof(ssl.io)` - # returns at once, without ever yielding. - eof(ssl.io) && return true + if pending === nothing && err === nothing + if ret == SSL_ERROR_NONE + return false + elseif ret == SSL_ERROR_WANT_WRITE + flush(ssl.io) + elseif ret == SSL_ERROR_WANT_READ + # if we get WANT_READ back, that means there were pending bytes + # to be processed, but not a full record, so we need to wait + # for additional bytes to come in before we can process. If the + # socket is at EOF they never will: the peer went away mid-record, + # so this is the end of the stream. Looping instead would spin, + # `haspending` stays true for the partial record and `eof(ssl.io)` + # returns at once, without ever yielding. + eof(ssl.io) && return true + end + continue end + (ret, pending, err) end + # processing the record produced something for the peer (a KeyUpdate reply, or + # the alert of a record that failed): send it, or fail, without holding + # `eoflock`. A writer parked on a peer that is not reading would otherwise hold + # every other reader up through this lock. Then go round again: whether the + # peek made bytes available is re-checked at the top. + finish_sslcall!(ssl, ret, pending, err) end bytesavailable(ssl) > 0 && return false return !isopen(ssl) @@ -917,12 +946,16 @@ function closesocket!(ssl::SSLStream, pending) try drain!(getfield(ssl, :data), pending) catch err + # the peer being gone is expected here; an interrupt or a cancellation of the + # task is not ours to swallow + err isa Union{Base.IOError, EOFError} || rethrow() @debug "SSL alert not sent" err - end - @async try - Base.close(ssl.io) - catch e - e isa Base.IOError || rethrow() + finally + @async try + Base.close(ssl.io) + catch e + e isa Base.IOError || rethrow() + end end return end diff --git a/test/runtests.jl b/test/runtests.jl index d68ebd2..53d4119 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -857,8 +857,12 @@ end catch ex ex end - @test timedwait(() -> istaskstarted(third_writer), 5.0) === :ok - yield() + # both have to be parked on the condition before one is cancelled: delivering an + # exception to a task that is still running (possible on another thread) is not + # allowed, and the cancellation has to land in the wait for the turn + cond = client.data.cond + waiting(task) = Base.@lock cond (task in cond.waitq) + @test timedwait(() -> waiting(cancelled_writer) && waiting(third_writer), 5.0) === :ok @test !istaskdone(cancelled_writer) @test !istaskdone(third_writer) # the second one gets cancelled while it waits for its turn From 7a73396a1c07dbe4afaa075dc66b6d571974441c Mon Sep 17 00:00:00 2001 From: Krystian Gulinski Date: Mon, 28 Sep 2026 13:11:34 +0200 Subject: [PATCH 13/25] fix: no close_notify from the finalizer; count queued writers for the test `CancelledWriter` looked for the queued writers in the condition's wait queue, which nightly no longer stores as tasks. `BIOStreamData` counts the tasks parked in `drain!` instead, under the condition's lock, and the test polls that. The finalizer called `close(ssl)`, which sends the close_notify, a socket write. Nightly's socket write path waits there, and a finalizer may not switch tasks: `NoCloseStream` printed "task switch not allowed from inside gc finalizer". Older versions only wait when the socket buffer is full, so the hazard was latent. The finalizer now calls `close(ssl, false)`: the SSL object is freed and the socket closed, nothing is written. Co-Authored-By: Claude Fable 5.1 --- src/ssl.jl | 15 ++++++++++++--- test/runtests.jl | 6 +++--- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/ssl.jl b/src/ssl.jl index 1677706..674335b 100644 --- a/src/ssl.jl +++ b/src/ssl.jl @@ -63,9 +63,11 @@ mutable struct BIOStreamData # tickets whose task was cancelled while waiting for its turn; passed over when # the turn reaches them, so no ticket goes unconsumed and parks every later one abandoned::Set{Int} + # tasks parked in `drain!` waiting for their turn + waiting::Int end -BIOStreamData(io::TCPSocket) = BIOStreamData(io, UInt8[], Threads.Condition(), 0, 0, Set{Int}()) +BIOStreamData(io::TCPSocket) = BIOStreamData(io, UInt8[], Threads.Condition(), 0, 0, Set{Int}(), 0) """ Ciphertext one SSL call produced, and its place in the socket write order. @@ -112,7 +114,12 @@ function drain!(data::BIOStreamData, pending::PendingWrite, owner=nothing) Base.@lock data.cond begin try while data.turn != pending.ticket - wait(data.cond) + data.waiting += 1 + try + wait(data.cond) + finally + data.waiting -= 1 + end end catch # cancelled while waiting (`schedule(task, ex; error=true)`, an interrupt): @@ -546,7 +553,9 @@ mutable struct SSLStream <: IO bio_write::BIO = BIO(data; finalize=false) ssl = SSL(ssl_context, bio_read, bio_write) x = new(ssl, ssl_context, bio_read, bio_write, io, ReentrantLock(), ReentrantLock(), false, data) - finalizer(close, x) + # no close_notify from the finalizer: sending it is a socket write, which can + # wait for the peer, and a finalizer may not switch tasks + finalizer(x -> close(x, false), x) return x end end diff --git a/test/runtests.jl b/test/runtests.jl index 53d4119..9cf2885 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -860,9 +860,9 @@ end # both have to be parked on the condition before one is cancelled: delivering an # exception to a task that is still running (possible on another thread) is not # allowed, and the cancellation has to land in the wait for the turn - cond = client.data.cond - waiting(task) = Base.@lock cond (task in cond.waitq) - @test timedwait(() -> waiting(cancelled_writer) && waiting(third_writer), 5.0) === :ok + data = client.data + queued() = Base.@lock data.cond data.waiting + @test timedwait(() -> queued() == 2, 5.0) === :ok @test !istaskdone(cancelled_writer) @test !istaskdone(third_writer) # the second one gets cancelled while it waits for its turn From a7a8429cb5bb038369d9e3e8a732562dc78315e3 Mon Sep 17 00:00:00 2001 From: Krystian Gulinski Date: Wed, 30 Sep 2026 09:39:30 +0200 Subject: [PATCH 14/25] Address review: close, abort and cancellation semantics, handshake timeouts Follow-up to the non-blocking write BIO, from review of the earlier commits and of quinnj's comments. Squashed from local iterations; the behaviour, by area: Closing - close(ssl) is graceful by default: writes issued on other tasks before it finish first, later ones are refused (iswritable says so), then the close_notify goes out behind them and the socket is closed with what is queued on it flushed. - close(ssl, false) aborts: the stream is marked closed at once, what was produced before still goes out in order, then the socket is closed. - Either kind is bounded by progress: an AbortWatch closes the socket outright once nothing has moved for CLOSE_GRACE / ABORT_GRACE seconds, failing a write parked on a peer that is not reading. A graceful close on a stream that lost a record ends as an abort, with no close_notify. - A failed SSL call closes and aborts the stream in one go under ssl.lock, and its alert still goes out; the error now carries OpenSSL's reason (e.g. "tlsv1 alert unknown ca") and leaves the thread's error queue empty. Write ordering and cancellation - take! hands out tickets without taking any lock but ssl.lock; drain! waits for its turn. A writer cancelled while waiting consumes its ticket; one cancelled inside the socket write has its socket closed outright (cut!) and its chunk kept alive until libuv lets go of it (no use-after-free). - Cleanup paths take their locks through `surely`: an exception thrown into a task while it waits leaves the rest of the cleanup to a library task instead of skipping it. Interrupts are held across cleanup sections; library tasks report an interrupt they could not pass on. Handshake - Sockets.connect and Sockets.accept take `timeout` (a deadline for the whole handshake, verification included); the stream is closed on any failure. - accept runs the whole handshake, as connect does; ssl_accept is gone (it could not send its output once the BIO only buffers). Robustness - Log calls on cleanup paths are @guarded, so a logger that throws cannot stop one. - A timer helper (`ticker`) replaces Timer callbacks, with interrupts held around each callback. - The finalizer aborts a dropped stream, or closes it gracefully if its socket outlived it. Tests - New testsets for concurrent readers and writers, cancelled and parked writers, graceful and abort closes against a stalled peer, handshake deadlines, alert delivery, interrupted cleanup, throwing loggers and lost records. Stall-prone reads run under deadlines (awaitpeer, awaitread, boundedread, boundedfetch) so a regression fails a testset instead of hanging the suite. Checked on Julia 1.6, 1.7, 1.8, 1.10, 1.12 and nightly, 1 and 4 threads; throughput of small writes and allocations per write unchanged. Co-Authored-By: Claude Opus 5.5 --- src/OpenSSL.jl | 83 ++- src/ssl.jl | 1694 +++++++++++++++++++++++++++++++++++++++------- test/runtests.jl | 1561 +++++++++++++++++++++++++++++++++++++----- 3 files changed, 2865 insertions(+), 473 deletions(-) diff --git a/src/OpenSSL.jl b/src/OpenSSL.jl index 976fb9a..f842ed1 100644 --- a/src/OpenSSL.jl +++ b/src/OpenSSL.jl @@ -1560,8 +1560,8 @@ mutable struct BIO # note that `data` must be held as a reference somewhere else # since it is not referenced by the BIO directly - # e.g. in SSLStream, we keep the `io` reference that is passed to - # the read/write BIOs + # e.g. SSLStream gives both its BIOs the same `BIOStreamData`, and keeps it + # in its `data` field ccall( (:BIO_set_data, libcrypto), Cvoid, @@ -3053,46 +3053,65 @@ Base.show(io::IO, evp_pkey::EvpPKey) = write(io, String(evp_pkey)) Error handling. """ function get_error()::String - # Create memory BIO - bio = BIO(BIOMethodMemory()) - - local error_msg::String - - # Check existing error messages stored in task TLS. + # what the thread's queue holds, and a message a call left in the task's local + # storage (see `update_tls_error_state`) ahead of it, taken off as it is read + queue = errorqueue() if haskey(task_local_storage(), :openssl_err) - # Copy existing error from task TLS. tls_msg = task_local_storage(:openssl_err) delete!(task_local_storage(), :openssl_err) + return "$(tls_msg) : $(queue)" + end + return queue +end - # Clear the error queue, print the error messages to the memory BIO. - ccall( - (:ERR_print_errors, libcrypto), - Cvoid, - (BIO,), - bio) - - bio_msg = String(bio_get_mem_data(bio)) - error_msg = "$(tls_msg) : $(bio_msg)" - else - # Clear the error queue, print the error messages to the memory BIO. +# the thread's OpenSSL error queue, printed into a memory BIO and so taken off the +# queue; cleared all the same should the print fail. The BIO made here by hand: the +# constructor reports a failure through `get_error`, which reads the queue here, and +# would go round, taking a task-local message on the way +function errorqueue()::String + ptr = ccall( + (:BIO_new, libcrypto), + Ptr{Cvoid}, + (BIOMethod,), + BIOMethodMemory()) + # nothing to print into (OpenSSL out of memory, most likely, which is then the very + # reason to report): the queue read entry by entry into a buffer of Julia's instead + ptr == C_NULL && return errorqueue_lines() + # the BIO freed through its pointer, whatever the wrapper made of it + try + bio = BIO(ptr) ccall( (:ERR_print_errors, libcrypto), Cvoid, (BIO,), bio) - - error_msg = String(bio_get_mem_data(bio)) + return String(bio_get_mem_data(bio)) + finally + clear_errors!() + ccall((:BIO_free, libcrypto), Cint, (Ptr{Cvoid},), ptr) + end +end + +# the thread's error queue as `ERR_error_string_n` gives each entry, one a line, taken +# off the queue; for when no BIO can be made to print it +function errorqueue_lines()::String + out = IOBuffer() + line = Vector{UInt8}(undef, 256) + try + while (code = ccall((:ERR_get_error, libcrypto), Culong, ())) != 0 + GC.@preserve line begin + ccall( + (:ERR_error_string_n, libcrypto), + Cvoid, + (Culong, Ptr{UInt8}, Csize_t), + code, line, length(line)) + println(out, unsafe_string(pointer(line))) + end + end + finally + clear_errors!() end - - # Read the formatted error messages from the memory BIO. - - # Ensure the queue is clear (if ERR_print_errors fails). - clear_errors!() - - # Free bio. - free(bio) - - return error_msg + return String(take!(out)) end function clear_errors!() diff --git a/src/ssl.jl b/src/ssl.jl index 674335b..f14b9d1 100644 --- a/src/ssl.jl +++ b/src/ssl.jl @@ -1,3 +1,16 @@ +# a log call on a cleanup path: nothing a logger throws (one that does not catch its own +# errors rethrows them) is taken for the cleanup's failure, nor stops what the cleanup +# does after it +macro guarded(ex) + return quote + try + $(esc(ex)) + catch + end + nothing + end +end + """ BIO Stream callbacks. """ @@ -32,21 +45,36 @@ end bio_set_read_retry(bio::BIO) = bio_set_flags(bio, BIO_FLAGS_READ | BIO_FLAGS_SHOULD_RETRY) bio_clear_flags(bio::BIO) = bio_set_flags(bio, 0x00) +""" + Ciphertext one SSL call produced, and its place in the socket write order. +""" +struct PendingWrite + chunk::Vector{UInt8} + ticket::Int +end + """ What the read and write BIOs of an `SSLStream` are given as their data. OpenSSL calls the write BIO from inside `SSL_write_ex`, `SSL_connect`, `SSL_accept` -and `SSL_shutdown`, all of which run with `ssl.lock` held, and `SSL_read_ex` needs that -same lock. Writing to the socket in the callback therefore blocks every read on the +and `SSL_shutdown`, and from `SSL_read_ex` and `SSL_peek_ex` too when a read produces +records of its own (a reply to a KeyUpdate, an alert), all of which run with `ssl.lock` +held. Writing to the socket in the callback therefore blocks every read on the connection for as long as the peer's receive window stays full, which deadlocks any traffic that saturates both directions at once. The callback only buffers; the task that made the SSL call takes what it produced with `take!` while it still holds -`ssl.lock`, and moves it to the socket with `drain!` after releasing it. +`ssl.lock`, and moves it to the socket with `drain!` after releasing it. Some sends are +not the caller's: a read's reply goes out from a task of its own (see +`finish_sslcall!`), the alert of a call that failed goes out with the abort (see +`abortlocked!`), and the close_notify of a dropped stream whose socket outlived it goes +out from the finalizer's task (see `finalize!`). Every SSL call runs under `ssl.lock` and takes its own output before releasing it, so -`buf` is empty whenever a call starts and only ever holds the records of the call in -progress: a reader never ends up writing a writer's ciphertext to the socket, and each -writer waits for its own bytes. +`buf` is empty whenever a call starts (a close cut short between producing its +close_notify and taking it leaves it there, for the abort that follows to empty) and +only ever holds the records of the call in progress: a reader never ends up writing a +writer's ciphertext to the socket, and each writer waits for its own bytes; and a +reader, its reply sent from a task of its own, never waits behind a writer either. """ mutable struct BIOStreamData io::TCPSocket @@ -56,26 +84,37 @@ mutable struct BIOStreamData buf::Vector{UInt8} # socket writes happen in the order the chunks were taken, so records leave in the # order OpenSSL made them: `take!` hands out a ticket and `drain!` waits for its - # turn. Guards `nextticket`, `turn` and `abandoned`. + # turn. Guards `turn`, `abandoned`, `waiting`, `lostfrom` and `cut`. cond::Threads.Condition + # the next ticket: handed out only by `take!`, under `ssl.lock`, which serialises + # every SSL call, and read there, or once an abort has marked the stream closed, + # after which no call makes records nor takes a ticket nextticket::Int turn::Int - # tickets whose task was cancelled while waiting for its turn; passed over when - # the turn reaches them, so no ticket goes unconsumed and parks every later one + # tickets given up before the turn reached them (their task cancelled while it + # waited, refused for a loss before them, or an abort's alert given up); passed over + # when the turn reaches them, so no ticket goes unconsumed and parks every later one abandoned::Set{Int} # tasks parked in `drain!` waiting for their turn waiting::Int + # the first ticket whose record did not reach the socket: its task was cancelled, + # its write failed, or the socket was cut (see `cut!`). Nothing from that ticket on + # may be sent, the peer would reject it at the gap in the sequence numbers, and + # that includes the close_notify: the stream can only be aborted. `typemax(Int)` + # until then. + lostfrom::Int + # an abort is under way (see `abortlocked!`); a second one has nothing to add. Set + # and read under `ssl.lock`, where every abort runs + aborted::Bool + # a graceful close sent its close_notify and handed the socket to its normal close: + # the finalizer has nothing to finish + handedoff::Bool + # the socket handle was closed outright (see `cut!`): no ticket is written any more, + # and whatever waits for one stops waiting. Under `cond` + cut::Bool end -BIOStreamData(io::TCPSocket) = BIOStreamData(io, UInt8[], Threads.Condition(), 0, 0, Set{Int}(), 0) - -""" - Ciphertext one SSL call produced, and its place in the socket write order. -""" -struct PendingWrite - chunk::Vector{UInt8} - ticket::Int -end +BIOStreamData(io::TCPSocket) = BIOStreamData(io, UInt8[], Threads.Condition(), 0, 0, Set{Int}(), 0, typemax(Int), false, false, false) """ take!(data::BIOStreamData) -> Union{PendingWrite, Nothing} @@ -87,13 +126,16 @@ output. """ function Base.take!(data::BIOStreamData) isempty(data.buf) && return nothing + # the buffer that takes over made first; then the ticket and the swap, together and + # with no wait, no call and no allocation among them, so nothing can land in + # between: no lock either, `ssl.lock` serialising the tickets (see `nextticket`). + # An interrupt at the allocation leaves the records where they are, for `@sslcall` + # to give up together with the stream + fresh = UInt8[] + ticket = data.nextticket + data.nextticket = ticket + 1 chunk = data.buf - data.buf = UInt8[] - ticket = Base.@lock data.cond begin - t = data.nextticket - data.nextticket += 1 - t - end + data.buf = fresh return PendingWrite(chunk, ticket) end @@ -104,16 +146,17 @@ end have reached the socket, or sees the error when they do not. Nothing to write costs nothing, so a reader never waits behind a writer that is blocked on the peer. -`owner`, when given, is the `SSLStream` the chunk belongs to: if it was closed while -this task waited for its turn, the chunk is not written and an `IOError` is thrown -instead. Records before it were lost, so sending it would only have the peer reject -it. The close paths pass no owner: the stream is closed by then by design. +Once a record was lost (see `lostfrom`), the chunks after it are refused with an +`IOError` instead of being written. """ -drain!(data::BIOStreamData, pending::Nothing, owner=nothing) = nothing -function drain!(data::BIOStreamData, pending::PendingWrite, owner=nothing) - Base.@lock data.cond begin - try - while data.turn != pending.ticket +drain!(data::BIOStreamData, pending::Nothing) = nothing +function drain!(data::BIOStreamData, pending::PendingWrite) + try + Base.@lock data.cond begin + while true + # refused (see `lostfrom`), whether before or once the turn is here + pending.ticket >= data.lostfrom && throw(Base.IOError("an earlier record was not sent", 0)) + data.turn == pending.ticket && break data.waiting += 1 try wait(data.cond) @@ -121,28 +164,127 @@ function drain!(data::BIOStreamData, pending::PendingWrite, owner=nothing) data.waiting -= 1 end end - catch - # cancelled while waiting (`schedule(task, ex; error=true)`, an interrupt): - # the ticket still has to be consumed, or the turn never gets past it and - # every later writer parks on it. It may already be ours if the notification - # and the cancellation raced. The record this chunk holds never reaches the - # peer, which makes the connection unusable; `drain!(::SSLStream)` closes it, - # so the writers still waiting fail on the socket instead of hanging. - if data.turn == pending.ticket - passturn!(data) - else - push!(data.abandoned, pending.ticket) - end - rethrow() end + catch + # refused, or cancelled (`schedule(task, ex; error=true)`, an interrupt) while + # taking the lock or waiting for the turn: the ticket still has to be consumed, + # or the turn never gets past it and every later writer parks on it. With an + # interrupt held back: taking the lock is a cancellation point where there are + # such, and a cancelled task would otherwise be stopped short of it + holdinterrupts(() -> abandon!(data, pending.ticket)) + rethrow() end + sent = false + # the write ended with libuv's own error status for it (the `IOError` Base makes of + # it, "write: ..." with the libuv code, which is negative): libuv finished the request, + # and holds no pointer into the chunk. Anything else ending it (a cancellation, any + # other exception thrown into the task, an interrupt, or this never being set) may + # leave the write queued. The message is Base's wording: were it ever reworded, + # failed writes would keep their chunks until their sockets close, a leak and no + # more, and `FailedWriteKeepsNothing` would say so + finished = false + failure = nothing try - owner === nothing || isopen(owner) || throw(Base.IOError("ssl is closed", 0)) - chunk = pending.chunk - GC.@preserve chunk unsafe_write(data.io, pointer(chunk), UInt(length(chunk))) + write(data.io, pending.chunk) + sent = true + catch ex + failure = ex + finished = ex isa Base.IOError && ex.code < 0 && startswith(ex.msg, "write:") + rethrow() finally - # always pass the turn on, or one failed write would park every later one - Base.@lock data.cond passturn!(data) + # bound anew for the closures: captured as they are, having been assigned in the + # `try`, they would be boxed on every write + let sent = sent, ticket = pending.ticket, keep = finished ? nothing : pending.chunk, + failure = failure + # The write failed, or the task was cancelled inside it. The record is lost + # either way, and with it the stream. A cancelled write is moreover still + # queued in libuv, with a pointer into the chunk that nothing keeps alive any + # more: closing the socket handle now cancels it, and the chunk is kept until + # it has. (A cancellation that lands just as the write completed replaces its + # result, so that record is counted lost, and the stream given up, though it + # got through: the conservative side of not being able to tell.) Here in the + # `finally` rather than in the `catch`, which a second interrupt could leave + # before it got this far; one landing on the very entry of the `finally` is as + # close as Julia lets this be shut, and is Base's own `uv_write` gap, the + # interrupted write's buffer left to its caller. + # + # And the turn is always passed on, or one failed write would park every later + # one: in the same hold as the cut, so that an interrupt held back during the + # cut, thrown as a hold ends, cannot come between them, and whatever the cut + # throws (an interrupt, or an exception thrown into the task; a failure it + # logs itself, with the write's error) + holdinterrupts() do + try + sent || cut!(data, keep, failure) + finally + passon!(data, ticket, sent) + end + end + end + end + return nothing +end + +# closes the socket handle outright, and ends whatever waits for a turn or for every +# ticket to be through; with an interrupt held back, so that both happen. Every close +# of the handle outright goes through this: a failed write, the handshake deadline, a +# watch giving up, an abort that failed. The close failing is logged, with `cause`, what +# the cut is for, if given (not the whole exception stack: on a caller's task that holds +# the caller's own errors too). What comes out is an interrupt, or an exception thrown +# into the task (see `surely`) +function cut!(data::BIOStreamData, keep=nothing, cause=nothing) + holdinterrupts() do + try + forceclose!(data.io, keep) + catch ex + # nothing in the close throws one, and the hold keeps a Ctrl-C off; should + # one come all the same, where a hold cannot keep it off, it is the task's + ex isa InterruptException && rethrow() + bt = catch_backtrace() + @guarded @error("OpenSSL: closing a socket outright failed", + exception=(ex, bt), cause) + finally + # recorded in the ticket state, not left to the socket's status, which a + # fallback close sets only later and without a word to these waiters: every + # ticket from the turn on is refused, which ends the turn waits, and `cut` + # ends the abort's wait for tickets to come through, a ticket never drained + # included. Whatever the close did + surely(data.cond) do + data.cut = true + lose!(data, data.turn) + end + end + end + return nothing +end + +# passes the turn on after a drain; a chunk not sent is given up as `abandon!` does +function passon!(data::BIOStreamData, ticket::Int, sent::Bool) + sent || return abandon!(data, ticket) + surely(() -> passturn!(data), data.cond) + return nothing +end + +# records that the chunk of `ticket` is not going to be sent, and wakes the tasks waiting +# for their turn so those now refused can leave; under `cond` +function lose!(data::BIOStreamData, ticket::Int) + data.lostfrom = min(data.lostfrom, ticket) + notify(data.cond) + return nothing +end + +# gives up on a ticket whose chunk will never be written. Consumes the ticket, right +# away when the turn is already on it, otherwise when the turn reaches it; and records +# the loss. Takes `cond` itself, `abandon!` may run after a cancelled `lock`; surely, as +# every cleanup takes its locks (see `surely`). +function abandon!(data::BIOStreamData, ticket::Int) + surely(data.cond) do + lose!(data, ticket) + if data.turn == ticket + passturn!(data) + else + push!(data.abandoned, ticket) + end end return nothing end @@ -158,6 +300,10 @@ function passturn!(data::BIOStreamData) return nothing end +# every ticket handed out is through, drained or given up; under `cond`, and on a stream +# marked closed, `nextticket` being otherwise `take!`'s, under `ssl.lock` +drained(data::BIOStreamData) = data.turn == data.nextticket + function on_bio_stream_read(bio::BIO, out::Ptr{Cchar}, outlen::Cint) try bio_clear_flags(bio) @@ -182,9 +328,10 @@ function on_bio_stream_write(bio::BIO, in::Ptr{Cchar}, inlen::Cint)::Cint if data isa BIOStreamData # buffer only; the caller of the SSL call this runs inside takes it with # `take!` and writes it to the socket with `drain!` once `ssl.lock` is free - n = length(data.buf) - resize!(data.buf, n + inlen) - GC.@preserve data unsafe_copyto!(pointer(data.buf, n + 1), Ptr{UInt8}(in), UInt(inlen)) + buf = data.buf + n = length(buf) + resize!(buf, n + inlen) + GC.@preserve buf unsafe_copyto!(pointer(buf, n + 1), Ptr{UInt8}(in), Int(inlen)) return inlen end written = unsafe_write(data::IO, in, inlen) @@ -476,6 +623,10 @@ function ssl_set_host(ssl::SSL, host) end end +# one round of the client side of the handshake. Internal: with the write BIO +# buffering, what it produces stays in the stream's buffer, for `@geterror` around it to +# take under `ssl.lock` and send after; `Sockets.connect(::SSLStream)` is the way to +# connect function ssl_connect(ssl::SSL) return ccall( (:SSL_connect, libssl), @@ -484,33 +635,21 @@ function ssl_connect(ssl::SSL) ssl) end -function ssl_accept(ssl::SSL) - if (ret = ccall( - (:SSL_accept, libssl), - Cint, - (SSL,), - ssl)) != 1 - throw(OpenSSLError(ret)) - end - - ccall( - (:SSL_set_read_ahead, libssl), - Cvoid, - (SSL, Cint), - ssl, - Int32(1)) - return nothing -end +# gone in 1.6.2: with the write BIO buffering, its handshake output never reached the +# socket, and working on the bare handle it had no way to send it +ssl_accept(::SSL) = error("ssl_accept was removed: use Sockets.accept(::SSLStream)") -""" - Shut down a TLS/SSL connection. -""" +# queues the close_notify. Internal, as `ssl_connect`: `close(::SSLStream)` is the way +# to shut a stream down function ssl_disconnect(ssl::SSL) - ccall( + ret = ccall( (:SSL_shutdown, libssl), Cint, (SSL,), ssl) + # a failure only reports, on the thread's error queue, where a later call would + # take it for its own + ret < 0 && clear_errors!() return nothing end @@ -542,9 +681,28 @@ mutable struct SSLStream <: IO # call `read` or `write` at the same time as per the thread in # https://mailing.openssl.users.narkive.com/HeNGlNAJ/openssl-and-multithreaded-programs lock::ReentrantLock + # held for the whole of a `write`: it goes to OpenSSL in chunks, with `lock` released + # between them, and the chunks of two writers must not interleave + wlock::ReentrantLock closed::Bool + # the writes in flight, and whether a graceful close has begun, in one word: the + # `CLOSING` bit and a count. A write counts itself in with a compare-and-swap that + # fails once the bit is set, so a write issued before the close is counted and goes + # through, whichever gets `wlock` first, and one issued after is refused at once + # without ever being counted: neither a writer in a loop, retrying or not, nor a + # parked one keeps the close waiting for good. The close sets the bit and waits on + # `closecond`, on `lock`, for the count to reach zero; a second close finds the bit + # set and returns. Should a count be left standing all the same, nothing moves, and + # the close's watch gives up and ends the wait + wstate::Threads.Atomic{Int} + closecond::Threads.Condition # buffers the ciphertext the BIO callbacks produce, see `BIOStreamData` data::BIOStreamData + # scratch for `SSL_peek_ex` in `eof`: written by the call, never read back, so one + # per stream is fine (the read count has to be per call, and the write count is, see + # `unsafe_read` and `unsafe_write`) + peekbuf::Base.RefValue{UInt8} + peekbytes::Base.RefValue{Csize_t} function SSLStream(ssl_context::SSLContext, io::TCPSocket) # Create a read and write BIOs. @@ -552,26 +710,316 @@ mutable struct SSLStream <: IO bio_read::BIO = BIO(data; finalize=false) bio_write::BIO = BIO(data; finalize=false) ssl = SSL(ssl_context, bio_read, bio_write) - x = new(ssl, ssl_context, bio_read, bio_write, io, ReentrantLock(), ReentrantLock(), false, data) - # no close_notify from the finalizer: sending it is a socket write, which can - # wait for the peer, and a finalizer may not switch tasks - finalizer(x -> close(x, false), x) + lk = ReentrantLock() + x = new(ssl, ssl_context, bio_read, bio_write, io, ReentrantLock(), lk, + ReentrantLock(), false, Threads.Atomic{Int}(0), Threads.Condition(lk), + data, Ref{UInt8}(0x00), Ref{Csize_t}(0)) + finalizer(finalize!, x) return x end end SSLStream(tcp::TCPSocket) = SSLStream(SSLContext(OpenSSL.TLSClientMethod()), tcp) +# `shielded` runs `f` out of reach of a cancellation of the caller's scope, where there +# is such a thing (from the Julia nightly on, an interrupt is one): tasks made in there do +# not inherit it, as Base does for its own. `holdinterrupts` runs `f` with an +# interrupt held back: the same there, where `disable_sigint` no longer holds one back +# (it warns instead), and `disable_sigint` elsewhere +@static if isdefined(Base, :CANCEL_TOKEN) + shielded(f) = Base.ScopedValues.with(f, Base.CANCEL_TOKEN => nothing) + holdinterrupts(f) = shielded(f) +else + shielded(f) = f() + holdinterrupts(f) = disable_sigint(f) +end + +# runs `f` in a task of its own. Not `@async`: from Julia 1.7 that pins the task that +# calls it to its thread for good, and these are called from readers and writers that +# may well have been spawned themselves. In the caller's thread pool, so that an +# interactive task's cleanup does not queue behind compute work. (The finalizer's +# `@async` is not pinned: a finalizer runs on no task of its own.) +@static if isdefined(Threads, :threadpool) + # a task made as `Threads.@spawn` makes it, not yet scheduled, for what must exist + # before anything is changed that it is to finish + function unscheduled(f) + pool = Threads.threadpool() === :interactive ? :interactive : :default + return shielded() do + task = Task(f) + task.sticky = false + Threads._spawn_set_thrpool(task, pool) + task + end + end +else + function unscheduled(f) + task = Task(f) + task.sticky = false + return task + end +end +background(f) = schedule(unscheduled(f)) + +# `f()` under `l`, for the cleanup sections, which have to get through; then `then`, if +# given, on its value, outside the lock. The wait for the lock is where an exception +# thrown into the task (`schedule(task, ex; error=true)`, which an interrupt hold does +# not keep off, as it does a Ctrl-C) can still land: then the section, and `then`, are +# left to a task of the library's, which nothing outside it can throw into, and the +# exception goes on at once. Returns what `then` returns, or the section's value +function surely(f, l; then=nothing) + try + lock(l) + catch + leave() do + local v = Base.@lock(l, f()) + then === nothing ? v : then(v) + end + rethrow() + end + r = try + f() + finally + unlock(l) + end + return then === nothing ? r : then(r) +end + +# runs `f` on a task of the library's, which nothing waits for, whole (see `runwhole`), +# a failure logged. Returns the task +leave(f) = background() do + try + runwhole(f) + catch ex + if ex isa InterruptException + # one `f` threw itself, the only kind `runwhole` passes on + @guarded @error "OpenSSL: a cleanup left to a task of the library was interrupted" exception=caught(ex) + else + @guarded @error "OpenSSL: a cleanup left to a task of the library failed" exception=caught(ex) + end + end +end + +# runs `f` whole, on a task of the library's, which nothing can throw into but a Ctrl-C: +# with an interrupt held back. One that lands before the hold began has `f` run as if it +# had not; one thrown as the hold ends, `f` done by then, is said to be lost. What `f` +# throws itself is thrown +function runwhole(f) + # 0 before the hold, 1 in `f`, 2 once `f` is done; made once, before the loop, so that + # a rerun (only ever from 0) sets nothing up outside the `try` + phase = Ref(0) + result = Ref{Any}(nothing) + while true + try + holdinterrupts() do + phase[] = 1 + result[] = f() + phase[] = 2 + end + return result[] + catch ex + if ex isa InterruptException + phase[] == 0 && continue + phase[] == 2 && (lostinterrupt(); return result[]) + end + rethrow() + end + end +end + +# `f()` on a task of the library's, through interrupts, for a step that can be run again +# and may wait with no bound (a close, a wait for a condition), so not in a hold: one +# landing in it is said to be lost, and `f` run again +function through(f) + while true + try + return f() + catch ex + ex isa InterruptException || rethrow() + lostinterrupt() + end + end +end + +# a timer that calls `cb(timer)` on each tick, as `Timer(cb, delay; interval)` does, from +# a task made with `background`: the task `Timer(cb, ...)` makes pins, up to Julia 1.11, +# the task that creates the timer to its thread. (A timer with no callback carries no +# cancellation scope: what it is waited on in decides, and `background` shields that +# task.) Closing the timer ends it; a one-shot timer closes itself once it has gone off. +# +# Each callback runs with interrupts held back, so that it is not stopped half way; the +# waits do not, since a hold there would keep Ctrl-C from the whole process while it +# idles on this task. An interrupt, wherever else it lands, is not meant for the timer: +# it is said to have been lost and the timer goes on, or a watch would stop and leave a +# parked writer unfailed. What becomes of a tick it cut short depends on the timer. A +# one-shot timer's callback runs again, and runs once the timer can no longer go off +# should the interrupt have hidden that it did: a one-shot callback must be safe to run +# after its owner closed the timer and again after it was cut short (the handshake +# deadline's is). A repeating timer's tick is dropped and the next one waited for: a +# watch run again at once would judge nothing to have moved in no time. An error in a +# callback is logged and ends the timer, closed +function ticker(cb, delay::Real; interval::Real=0) + timer = Timer(delay; interval=interval) + background() do + # :idle, waiting for a tick; :due, a tick the callback has yet to run to its end + # for; :done, a one-shot timer's callback ran; :failed, a callback failed. Set + # within the callback's hold where it is the callback's outcome, so that an + # interrupt held back meanwhile, thrown as the hold ends, cannot keep it unset + state = Ref(:idle) + interrupted = false + try + while true + try + if interrupted + interrupted = false + lostinterrupt() + if interval == 0 + # a one-shot timer that can no longer go off may have gone + # off unseen, the interrupt landing in the wait as it did + state[] === :idle && !isopen(timer) && (state[] = :due) + else + state[] === :due && (state[] = :idle) + end + end + tickloop(cb, timer, interval, state) + return + catch ex + # only noted here, handled at the top of the next round: little + # enough happens in a `catch` for another interrupt to land in it + ex isa InterruptException || rethrow() + interrupted = true + end + end + catch ex + @guarded @error "OpenSSL: timer failed" exception=caught(ex) + finally + close(timer) + end + end + return timer +end + +# the ticks, until the timer is done with. Every way out is checked at the top of each +# round, where an interrupt that came in anywhere cannot have skipped it +function tickloop(cb, timer::Timer, interval::Real, state) + while true + state[] in (:failed, :done) && return + # a repeating timer closed by its owner is done with, whatever tick raced the + # close (`wait` still returns for a tick set before it) + interval == 0 || isopen(timer) || return + if state[] === :idle + open = try + wait(timer) + true + catch ex + ex isa EOFError || rethrow() + false + end + if open && (interval == 0 || isopen(timer)) + state[] = :due + elseif open + return + else + # closed: by its owner, or by itself as a one-shot timer that went off. + # Up to Julia 1.13 a first wait that meets the going off can see the timer + # closed without seeing it set; a tick that happened is not lost. Only + # for a one-shot timer: a repeating one closed by its owner is done with, + # whatever tick raced the close + interval == 0 && timerfired(timer) && (state[] = :due) + state[] === :due || return + end + end + runheld(cb, timer, interval, state) + end +end + +# what a `catch` in the ticker's own task logs of `ex`, the exception it is handling: the +# task's whole exception stack where Julia keeps it (1.7 on), so that an error thrown in +# a `finally` while another was on its way shows that one as its cause. Only there: on +# a caller's task the stack holds whatever the caller's own code is handling, which is +# no cause of the library's error +caught(ex) = @static VERSION >= v"1.7" ? current_exceptions() : (ex, catch_backtrace()) + +# one callback, held against interrupts, its outcome recorded within the hold. An +# interrupt out of the callback itself leaves the tick due, for `ticker` to decide on +function runheld(cb, timer::Timer, interval::Real, state) + holdinterrupts() do + try + cb(timer) + state[] = interval == 0 ? :done : :idle + catch ex + ex isa InterruptException && rethrow() + @guarded @error "OpenSSL: timer callback failed" exception=caught(ex) + state[] = :failed + end + end + return nothing +end + +# says that an interrupt landed in a task of the library and was not passed on; with +# further interrupts held back while it does, and those, as anything a logger throws, +# swallowed after +function lostinterrupt() + @guarded holdinterrupts() do + @warn "OpenSSL: an interrupt landed in a task of the library and was not passed on (nor any that came while this was said)" + end + return +end + +# `set` became an atomic field during Julia 1.8's development, a plain one before, and +# no version number tells for the 1.8 prereleases: the atomic read is tried the first +# time, and what it found is kept +const TIMERSET_ATOMIC = Threads.Atomic{Int8}(-1) +function timerfired(t::Timer) + atomic = TIMERSET_ATOMIC[] + if atomic < 0 + try + set = getfield(t, :set, :acquire) + TIMERSET_ATOMIC[] = 1 + return set + catch ex + ex isa InterruptException && rethrow() + TIMERSET_ATOMIC[] = 0 + atomic = Int8(0) + end + end + return atomic == 1 ? getfield(t, :set, :acquire) : getfield(t, :set) +end + +# the finalizer. A stream that was aborted, or closed gracefully and handed to its +# socket's normal close, is left alone: that close may still be flushing, and needs +# nothing of the stream. One that was dropped without being closed is closed from a task: a +# finalizer may not wait, as a contended `ssl.lock` would make it, but the task may. No +# writer can be parked on it, the writer's task would keep it alive. The socket is as a +# rule collected along with the stream, and its own finalizer closes the handle before +# the task runs: then there is no close_notify to send and the stream is aborted, which +# frees the SSL object and nothing more. Only a socket that outlived the sweep gets the +# graceful close. A dropped stream is torn down, not shut down: close it to shut it +# down. (Collected as the process exits, the task never runs and the SSL object is not +# freed; the OS reclaims it with everything else.) A stream marked closed but neither +# aborted nor handed to the socket's normal close was closed gracefully and stopped on +# the way, a close under way keeping the stream alive: it is aborted, so that its socket +# does not stay open +function finalize!(ssl::SSLStream) + if getfield(ssl, :closed) + data = getfield(ssl, :data) + data.aborted || data.handedoff || @async close(ssl, false) + return + end + @async socketclosed(getfield(ssl, :io)) ? close(ssl, false) : close(ssl) + return +end + # backwards compat Base.getproperty(ssl::SSLStream, nm::Symbol) = nm === :bio_read_stream ? ssl : getfield(ssl, nm) function drain!(ssl::SSLStream, pending) try - drain!(getfield(ssl, :data), pending, ssl) + drain!(getfield(ssl, :data), pending) catch # a failed socket write used to surface through the BIO callback as an SSL # error, which closed the stream; keep that so `isopen` does not report a - # connection whose ciphertext never reached the peer as usable + # connection whose ciphertext never reached the peer as usable; the abort holds + # interrupts back itself, so that a cancelled task still gets it done close(ssl, false) rethrow() end @@ -579,9 +1027,22 @@ end Base.isreadable(ssl::SSLStream)::Bool = isopen(ssl) && isreadable(ssl.io) Base.isopen(ssl::SSLStream)::Bool = Base.@lock(ssl.lock, !ssl.closed) -Base.iswritable(ssl::SSLStream)::Bool = isopen(ssl) && isopen(ssl.io) +# not while a graceful close is under way, which refuses new writes; the stream is still +# open then, and readable +Base.iswritable(ssl::SSLStream)::Bool = + Base.@lock(ssl.lock, !ssl.closed) && !closing(ssl) && isopen(ssl.io) @noinline throwio(op) = throw(Base.IOError("$op requires ssl to be open", 0)) +# the message of an SSL call that failed with `code`: its name, and what the thread's +# OpenSSL error queue holds of why, which reading takes off the queue, where a later +# call would otherwise take it for its own. The thread's queue alone: not `get_error`, +# which would also take an unrelated error another call left for its task +function sslcallerror(code) + name = OpenSSLError(code).msg + queue = rstrip(errorqueue()) + return isempty(queue) ? name : string(name, ": ", queue) +end + # this is a macro, but should be a function, but closures are stupid slow # we use this to standardize the error handling for all of the SSL_*_ex functions: # make the ccall under `ssl.lock`, check the error queue, and take the ciphertext the @@ -589,66 +1050,127 @@ Base.iswritable(ssl::SSLStream)::Bool = isopen(ssl) && isopen(ssl.io) # `finish_sslcall!` once every lock that must not be held across the socket write is # released; `@geterror` is the two together. macro sslcall(ssl, op, expr) + # the temporaries are gensyms: the whole quote is escaped, and plain names would + # clobber a caller's locals of the same name + _err, _ret, _r, _e, _pending, _ended = + gensym.(("err", "ret", "r", "e", "pending", "ended")) esc(quote - local _err = nothing - local _ret = SSL_ERROR_NONE - local _pending = Base.@lock $ssl.lock begin + local $_err = nothing + local $_ret = SSL_ERROR_NONE + # the call ends the stream (it failed, or the peer closed): set first in each such + # branch, before anything that allocates, for the rest and the catch to go by + local $_ended = false + local $_pending = Base.@lock $ssl.lock begin # check that SSL is still open before ccall $ssl.closed && throwio($op) # clear the current error queue before openssl ccall clear_errors!() # do the ccall - _r = $expr - # we want to return one of our SSL return codes, regardless of error - # SSL_peek_ex, SSL_write_ex, SSL_connect, SSL_accept and SSL_read_ex all - # return 1 on success - if _r != 1 - _e = get_error($ssl.ssl, _r) - if _e == SSL_ERROR_ZERO_RETURN - # the peer sent a close_notify, so no more reading is possible - _err = Base.IOError("unexpected EOF", 0) - elseif _e == SSL_ERROR_NONE || _e == SSL_ERROR_WANT_READ || _e == SSL_ERROR_WANT_WRITE - # WANT_READ: we need to read more data from the underlying socket - # WANT_WRITE: we need to write more data to the underlying socket; - # we don't expect to ever see this since we set up our SSL - # to do auto TLS (re)negotiation - _ret = _e + $_r = $expr + # the rest in a `try`: cut short (an interrupt at an allocation) after the + # call, what the call left in the buffer is not to go out with another call's, + # nor the stream to stay open. The records of a call that did not fail are + # lost, and the stream with them, as for a write cancelled under way (see + # `drain!`): dropped, and the stream aborted under `ssl.lock` still, so that no + # later call makes records after the gap; what a call that ended the stream + # produced (a failed call's alert, or anything a call meeting the peer's + # close_notify made) goes out with the abort, as it would have. A stream + # aborted already takes nothing more + try + # we want to return one of our SSL return codes, regardless of error + # SSL_peek_ex, SSL_write_ex, SSL_connect, SSL_accept and SSL_read_ex all + # return 1 on success + if $_r != 1 + $_e = get_error($ssl.ssl, $_r) + if $_e == SSL_ERROR_ZERO_RETURN + # the peer sent a close_notify, so no more reading is possible + $_ended = true + $_err = Base.IOError("unexpected EOF", 0) + elseif $_e == SSL_ERROR_NONE || $_e == SSL_ERROR_WANT_READ || $_e == SSL_ERROR_WANT_WRITE + # WANT_READ: we need to read more data from the underlying socket + # WANT_WRITE: we need to write more data to the underlying socket; + # we don't expect to ever see this since we set up our SSL + # to do auto TLS (re)negotiation + $_ret = $_e + else + # this is usually some other kind of error, like a protocol error + # or OS-level IO error, just close the SSL connection and throw + # notably, the openssl docs say we should *not* call ssl_disconnect + # in this case, hence the `false` arg to close + $_ended = true + $_err = Base.IOError(sslcallerror($_e), 0) + end + end + if !$_ended + take!(getfield($ssl, :data)) else - # this is usually some other kind of error, like a protocol error - # or OS-level IO error, just close the SSL connection and throw - # notably, the openssl docs say we should *not* call ssl_disconnect - # in this case, hence the `false` arg to close - _err = Base.IOError(OpenSSLError(_e).msg, 0) + # closed and aborted under the lock we already hold, in one go; the + # abort sends the alert OpenSSL queued for the peer, from a task + abortlocked!($ssl) end - end - if _err === nothing - take!(getfield($ssl, :data)) - else - # close under the lock we already hold; what comes back is the alert - # OpenSSL queued for the peer, sent by `finish_sslcall!` once the lock - # is released - closelocked!($ssl, false) + catch + # in a hold of its own from here, `abortlocked!`'s beginning only with its + # call; the buffer never handed out, emptied in place, which cannot fail + # (what the closure needs bound anew: captured as they are, assigned in + # the section, they would be boxed on every call) + let failed = $_ended + holdinterrupts() do + failed || empty!(getfield($ssl, :data).buf) + abortlocked!($ssl) + end + end + rethrow() end end - (_ret, _pending, _err) + ($_ret, $_pending, $_err) end) end -# the second half of an SSL call: send the alert and throw when it failed, otherwise +# the second half of an SSL call: throw when it failed (the stream was closed and +# aborted in `@sslcall` already, the abort sending the alert), otherwise # write what it produced to the socket and hand back its return code. The write BIO # only buffers, so this is where the socket write happens, and the caller waits for the # peer here rather than under `ssl.lock`. -function finish_sslcall!(ssl::SSLStream, ret::SSLErrorCode, pending, err) - if err !== nothing - closesocket!(ssl, pending) - throw(err) +# `detach` is for reads: what they produce is a reply to the peer that the reader has +# no reason to wait for, and waiting would put the reader behind a writer parked on a +# peer that is not reading; it is sent from a task, in its ticket's order like anything +# else. +function finish_sslcall!(ssl::SSLStream, ret::SSLErrorCode, pending, err; detach::Bool=false) + # failed: the stream was closed and aborted in `@sslcall` already + err === nothing || throw(err) + if detach && pending !== nothing + background() do + try + drain!(ssl, pending) + catch ex + # `drain!` closed the stream; the reader finds that out on its next call + @guarded @debug "SSL reply to the peer not sent" ex + end + end + else + drain!(ssl, pending) end - drain!(ssl, pending) return ret end -macro geterror(ssl, op, expr) - esc(:(finish_sslcall!($ssl, (@sslcall $ssl $op $expr)...))) +macro geterror(ssl, op, expr, detach=false) + esc(:(finish_sslcall!($ssl, (@sslcall $ssl $op $expr)...; detach=$detach))) +end + +# waits for bytes on the socket for an SSL call that asked for more, and says whether +# the socket is at its EOF instead. EOF does not close the stream, no more than `eof` on +# a socket does: the peer may have shut down its side only, and a reply may still be +# due; the handshakes, which have nothing usable then, close it themselves. An error on +# the socket, a reset say, is the end of the transport and the stream with it: closed +# here, and rethrown, so that the next call does not run into the same error again. +function socketeof(ssl::SSLStream) + try + return eof(ssl.io) + catch ex + ex isa Base.IOError || rethrow() + close(ssl, false) + rethrow() + end end # the write BIO buffers the whole output of one `SSL_write_ex` before `drain!` moves it @@ -656,83 +1178,246 @@ end const SSL_WRITE_CHUNK = UInt(1 << 20) function Base.unsafe_write(ssl::SSLStream, in_buffer::Ptr{UInt8}, in_length::UInt) - nwritten = 0 - # per call, not per stream: `@geterror` writes to the socket after releasing - # `ssl.lock`, and another task's `SSL_write_ex` would overwrite a shared count - # before it is read back below - writebytes = Ref{Csize_t}(0) - while nwritten < in_length - # SSL_write_ex writes all or nothing without SSL_MODE_ENABLE_PARTIAL_WRITE, so a - # retry after WANT_READ/WANT_WRITE resubmits the same chunk - chunk = min(in_length - nwritten, SSL_WRITE_CHUNK) - ret = @geterror ssl :unsafe_write ccall( - (:SSL_write_ex, libssl), - Cint, - (SSL, Ptr{Cvoid}, Csize_t, Ptr{Csize_t}), - ssl.ssl, - in_buffer + nwritten, - chunk, - writebytes - ) - if ret == SSL_ERROR_NONE - nwritten += writebytes[] - elseif ret == SSL_ERROR_WANT_WRITE - flush(ssl.io) - elseif ret == SSL_ERROR_WANT_READ - # this means write is waiting for more data from the underlying socket - # so call eof on the socket to wait for more bytes to come in - eof(ssl.io) && throw(EOFError()) + # nothing to write: nothing to refuse either, whatever state the stream is in + in_length == 0 && return 0 + counted = Ref(false) + try + # counted in, for a graceful close to wait for, or refused, should one have + # begun; either way before waiting for the writer lock behind a writer that may + # be parked. With an interrupt held back until the flag says which, so that + # the count goes down again whatever ends the write. (With `disable_sigint`, the + # hold has to begin and end with no `try` in between: leaving a `try` puts it + # back as it was on entry) + holdinterrupts(() -> countin!(ssl, counted)) + # refused: without taking `ssl.lock`, which a writer retrying in a loop would + # otherwise keep from the close; `closed` read as is only picks the message, the + # stream reporting open until the close is through + if !counted[] + ssl.closed && throwio(:unsafe_write) + throw(Base.IOError("unsafe_write: the stream is being closed", 0)) + end + # counted: a stream closed by an abort refuses it too + Base.@lock(ssl.lock, ssl.closed) && throwio(:unsafe_write) + # one writer at a time, from the first chunk to the last, so that a write arrives + # in one piece however many chunks it takes, as it did with one `SSL_write_ex` + # for the whole of it. Readers do not take this lock + Base.@lock ssl.wlock begin + nwritten = 0 + # per call, as `unsafe_read`'s: under `ssl.wlock` a shared one would be safe + # too, but a field would be one more thing kept in step with the lock + writebytes = Ref{Csize_t}(0) + while nwritten < in_length + # SSL_write_ex writes all or nothing without SSL_MODE_ENABLE_PARTIAL_WRITE, so a + # retry after WANT_READ/WANT_WRITE resubmits the same chunk + chunk = min(in_length - nwritten, SSL_WRITE_CHUNK) + ret = @geterror ssl :unsafe_write ccall( + (:SSL_write_ex, libssl), + Cint, + (SSL, Ptr{Cvoid}, Csize_t, Ptr{Csize_t}), + ssl.ssl, + in_buffer + nwritten, + chunk, + writebytes + ) + if ret == SSL_ERROR_NONE + nwritten += Base.bitcast(Int, writebytes[]) + elseif ret == SSL_ERROR_WANT_WRITE + flush(ssl.io) + elseif ret == SSL_ERROR_WANT_READ + # this means write is waiting for more data from the underlying socket + # so call eof on the socket to wait for more bytes to come in + socketeof(ssl) && throw(EOFError()) + end + end end + finally + counted[] && endwrite!(ssl) end return Base.bitcast(Int, in_length) end -function Sockets.connect(ssl::SSLStream; require_ssl_verification::Bool=true) +# the sign bit, whatever the width of `Int`; the count takes the bits below it +const CLOSING = typemin(Int) +const INFLIGHT = typemax(Int) + +closing(ssl::SSLStream) = ssl.wstate[] & CLOSING != 0 +writesinflight(ssl::SSLStream) = ssl.wstate[] & INFLIGHT + +# counts a write in, unless a graceful close has begun; says which in `counted` +function countin!(ssl::SSLStream, counted) while true - ret = @geterror ssl :connect ssl_connect(ssl.ssl) - if ret == SSL_ERROR_NONE - break - elseif ret == SSL_ERROR_WANT_READ - # this means connect is waiting for more data from the underlying socket - # so call eof on the socket to wait for more bytes to come in - eof(ssl.io) && throw(EOFError()) - else - throw(Base.IOError(OpenSSLError(ret).msg, 0)) + v = ssl.wstate[] + v & CLOSING == 0 || return + if Threads.atomic_cas!(ssl.wstate, v, v + 1) == v + counted[] = true + return end end +end - # Check the certificate. - if require_ssl_verification - Base.@lock ssl.lock begin - ssl.closed && throwio(:verify_result) - if (ret = ccall( - (:SSL_get_verify_result, libssl), - Cint, - (SSL,), - ssl.ssl)) != 0 - throw(OpenSSLError(unsafe_string(ccall( - (:X509_verify_cert_error_string, libcrypto), - Ptr{UInt8}, - (Cint,), - ret)))) +# counts a write out, then tells the close, under the lock the condition needs. With an +# interrupt held back, so that neither is cut short once begun; and never below zero: +# from there it would borrow through the `CLOSING` bit, clear it and leave a count no +# close could wait out. Not an error to throw, being called from a `finally` where it +# would replace the write's own error: it is logged. An interrupt that lands before +# this leaves the close to find out when its watch gives up +function endwrite!(ssl::SSLStream) + # all of it: the lock and the log are cancellation points where there are such + holdinterrupts() do + countout!(ssl) || + @guarded @error "OpenSSL: a write counted out that was never counted in" + surely(() -> notify(ssl.closecond), ssl.lock) + end + return +end + +function countout!(ssl::SSLStream) + while true + v = ssl.wstate[] + v & INFLIGHT == 0 && return false + Threads.atomic_cas!(ssl.wstate, v, v - 1) == v && return true + end +end + +""" + Sockets.connect(ssl::SSLStream; require_ssl_verification=true, timeout=Inf) + +Runs the client side of the TLS handshake on `ssl` and returns once it is complete, the +peer's certificate verified unless `require_ssl_verification` is false. Throws +`EOFError` when the peer goes away first, `IOError` when the handshake fails, `IOError` +once `timeout` seconds have passed since the call began, however far the handshake got, +and `OpenSSLError` when the peer's certificate does not verify (in a context that +verifies during the handshake itself, that is a failed handshake, an `IOError`); the +stream is closed in each of those cases. `timeout` is a positive number of seconds, or +`Inf` for none. +""" +function Sockets.connect(ssl::SSLStream; require_ssl_verification::Bool=true, timeout::Real=Inf) + # the peer's certificate checked as the handshake's last step, within its guard: a + # stream whose peer was not checked, whatever ended the check early, an interrupt + # included, is not to be used, and is closed with the rest of a half-done handshake + verify = require_ssl_verification ? (() -> verifypeer!(ssl)) : nothing + handshake!(ssl, :connect, timeout; finish=verify) do + @geterror ssl :connect ssl_connect(ssl.ssl) + end + return +end + +function verifypeer!(ssl::SSLStream) + failure = Base.@lock ssl.lock begin + ssl.closed && throwio(:verify_result) + ret = ccall( + (:SSL_get_verify_result, libssl), + Cint, + (SSL,), + ssl.ssl) + ret == 0 ? nothing : unsafe_string(ccall( + (:X509_verify_cert_error_string, libcrypto), + Ptr{UInt8}, + (Cint,), + ret)) + end + # get peer certificate + if failure === nothing && get_peer_certificate(ssl) === nothing + failure = "No peer certificate" + end + failure === nothing || throw(OpenSSLError(failure)) + return +end + +# runs a handshake to completion: `step` does one round of it (the ccall under +# `@geterror`) and returns the code; more bytes are waited for on the socket, whose EOF +# ends the stream. Then read ahead is set: a recommended optimization when an SSL +# connection is only ever read from sequentially, which it is, there being no internal +# buffering of decrypted bytes. `finish`, if given, is the last step: inside the guard +# that closes a half-done stream, and under the deadline, the handshake being done only +# once it is. It runs on a stream that the deadline or another task may close at any +# point, so it must take `ssl.lock` and check `ssl.closed` before it touches the SSL +# object, as `verifypeer!` does; its own error names its own op. +# +# `timeout` is one deadline for the whole handshake. When it passes, the timer aborts +# the stream, which wakes a round waiting on the socket and fails one stuck writing to +# a peer that does not read, and notes that it did so the error says so. Whether the +# handshake completed first is decided under `ssl.lock`, in the same section that marks +# the stream closed, since a timer that went off as the handshake completed still runs +# its callback after `close(timer)`. `Timer` counts on the monotonic clock. +function handshake!(step, ssl::SSLStream, op::Symbol, timeout::Real; finish=nothing) + # positive seconds, or none. Zero is refused rather than read as either "none" or + # "passed already", both of which some callers would mean; and `Timer` cannot count + # past about 1e16 seconds, so from 1e9 on (over thirty years) it is none as well + timeout > 0 || + throw(ArgumentError("$op: timeout must be a positive number of seconds or Inf, got $timeout")) + timeout >= 1e9 && (timeout = Inf) + # :running, then :done or :timedout, whichever comes first under `ssl.lock` + state = Ref(:running) + timer = timeout == Inf ? nothing : ticker(timeout) do _ + cause = nothing + try + Base.@lock ssl.lock begin + if state[] === :running && !ssl.closed + state[] = :timedout + abortlocked!(ssl) + end end + catch ex + ex isa InterruptException || (cause = ex) + rethrow() + finally + # timed out, by this run or by one that did not get to the end, whose end is + # still to do (done, or closed because the handshake failed or someone closed + # it, is not the deadline's to act on). Read outside the lock: only these + # runs, one after another in the ticker's task, ever set :timedout. Nothing a + # failed handshake produced is worth waiting for: the socket goes at once, + # which also fails a round parked writing to a peer that does not read; + # whatever the abort did, which the ticker then hears of, and which the cut + # names should it fail too + state[] === :timedout && cut!(getfield(ssl, :data), nothing, cause) end - # get peer certificate - cert = get_peer_certificate(ssl) - cert === nothing && throw(OpenSSLError("No peer certificate")) end - - # set read ahead; this is a recommended optimization when we can guarantee - # that an SSL connection will only ever be read from sequentially, which we do - # by not doing any internal buffering - Base.@lock ssl.lock begin - ssl.closed && throwio(:read_ahead) - ccall( - (:SSL_set_read_ahead, libssl), - Cvoid, - (SSL, Cint), - ssl.ssl, - Cint(1)) + try + while true + ret = step() + if ret == SSL_ERROR_NONE + break + elseif ret == SSL_ERROR_WANT_READ + # more bytes from the peer are needed, wait for them; the catch below + # closes the stream should the peer be gone + socketeof(ssl) && throw(EOFError()) + else + # WANT_WRITE cannot happen, the write BIO takes everything + throw(Base.IOError("$op: unexpected $ret from the handshake", 0)) + end + end + # the last step still under the deadline, which it counts toward (checking for a + # closed stream itself, see above) + finish === nothing || finish() + Base.@lock ssl.lock begin + state[] === :running && (state[] = :done) + # closed meanwhile: by the deadline, which the catch reports as such, or by + # another task. Or the deadline passed with the stream somehow still open (its + # abort having failed): the deadline decides, not what the abort left + (ssl.closed || state[] === :timedout) && throwio(op) + ccall( + (:SSL_set_read_ahead, libssl), + Cvoid, + (SSL, Cint), + ssl.ssl, + Cint(1)) + end + catch ex + # whatever ended the handshake, an interrupt included, a half-done stream is of + # no use: it is closed. Then, the abort makes the round under way fail with + # whatever it fails with, and the deadline is the cause; anything else is not + # ours to replace. Nor is it replaced by the abort failing, which is logged. An + # interrupt held off the abort is thrown once it is done; an exception thrown into + # the task while the abort waits for the lock goes on at once, the abort left to a + # task of the library's (see `abortsurely!`) + close(ssl, false) + if ex isa Union{Base.IOError, EOFError} && Base.@lock(ssl.lock, state[] === :timedout) + throw(Base.IOError("$op: the handshake timed out", 0)) + end + rethrow() + finally + timer === nothing || close(timer) end return end @@ -754,33 +1439,28 @@ function hostname!(ssl::SSLStream, host) end end -function Sockets.accept(ssl::SSLStream) - while true - ret = @geterror ssl :accept ccall( +""" + Sockets.accept(ssl::SSLStream; timeout=Inf) + +Runs the server side of the TLS handshake on `ssl` and returns once it is complete. +Throws `EOFError` when the peer goes away first, `IOError` when the handshake fails, +and `IOError` once `timeout` seconds have passed since the call began, however far the +handshake got; the stream is closed in each of those cases. `timeout` is a positive +number of seconds, or `Inf` for none. + +Before 1.6.2 the call did one round of the handshake and threw `OpenSSLError` whenever +it needed more bytes from the peer, and the caller retried. Those loops still work, they +get the completed handshake on the first call, but they no longer see `OpenSSLError`: +the errors are `EOFError` and `IOError` now. A deadline such a loop enforced between +attempts is what `timeout` is for. +""" +function Sockets.accept(ssl::SSLStream; timeout::Real=Inf) + handshake!(ssl, :accept, timeout) do + @geterror ssl :accept ccall( (:SSL_accept, libssl), Cint, (SSL,), ssl.ssl) - if ret == SSL_ERROR_NONE - break - elseif ret == SSL_ERROR_WANT_READ - # this means accept is waiting for more data from the underlying socket - # so call eof on the socket to wait for more bytes to come in - eof(ssl.io) && throw(EOFError()) - else - throw(Base.IOError(OpenSSLError(ret).msg, 0)) - end - end - - # see `connect` - Base.@lock ssl.lock begin - ssl.closed && throwio(:read_ahead) - ccall( - (:SSL_set_read_ahead, libssl), - Cvoid, - (SSL, Cint), - ssl.ssl, - Cint(1)) end return end @@ -790,9 +1470,13 @@ end """ function Base.unsafe_read(ssl::SSLStream, buf::Ptr{UInt8}, nbytes::UInt) nread = 0 - # per call, see `unsafe_write` + # per call, not per stream: readers take no lock across their rounds, and a count + # shared by the stream, overwritten by another reader's call before this one read it + # back, would lose or duplicate bytes, or count bytes never read (and return more + # than `nbytes`) readbytes = Ref{Csize_t}(0) while nread < nbytes + # `true`: what the read produces goes out from a task, see `finish_sslcall!` ret = @geterror ssl :unsafe_read ccall( (:SSL_read_ex, libssl), Cint, @@ -801,13 +1485,13 @@ function Base.unsafe_read(ssl::SSLStream, buf::Ptr{UInt8}, nbytes::UInt) buf + nread, nbytes - nread, readbytes - ) + ) true if ret == SSL_ERROR_NONE nread += Base.bitcast(Int, readbytes[]) elseif ret == SSL_ERROR_WANT_READ - # this means write is waiting for more data from the underlying socket + # this means read is waiting for more data from the underlying socket # so call eof on the socket to wait for more bytes to come in - eof(ssl.io) && throw(EOFError()) + socketeof(ssl) && throw(EOFError()) elseif ret == SSL_ERROR_WANT_WRITE flush(ssl.io) end @@ -850,10 +1534,10 @@ end function Base.eof(ssl::SSLStream)::Bool bytesavailable(ssl) > 0 && return false - # per call, see `unsafe_write` - peekbuf = Ref{UInt8}(0x00) - peekbytes = Ref{Csize_t}(0) while isopen(ssl) + # the peek runs under `eoflock`; what it produced is sent after the lock is + # released (see the end of the loop), so the result has to come out of the block + ret, pending, err = Base.@lock ssl.eoflock begin # note that care needs to be taken here to avoid a potential bad # race condition; for SSLStream, we have to manage the state of # the underlying socket having available bytes *and* whether they've @@ -864,7 +1548,6 @@ function Base.eof(ssl::SSLStream)::Bool # tasks are blocked calling eof on the socket or waiting on eoflock, so # we avoid the races and keep things orderly by only allowing one task # to make the eof call and kick off byte processing at a time. - Base.@lock ssl.eoflock begin # check condition now that we have eoflock since another task may have # succeeded in getting bytes processed isopen(ssl) || return true @@ -872,8 +1555,9 @@ function Base.eof(ssl::SSLStream)::Bool # no processed bytes available, check if there are unprocessed bytes if !haspending(ssl) # no unprocessed bytes, call eof to get more unprocessed - if eof(ssl.io) && !haspending(ssl) - # if eof and there are no pending, then we are eof + if socketeof(ssl) && !haspending(ssl) + # if eof and there are no pending, then we are eof. Not closed: as + # with `eof` on a socket, the peer may have shut down its side only return true end end @@ -883,11 +1567,11 @@ function Base.eof(ssl::SSLStream)::Bool ret, pending, err = @sslcall ssl :peek ccall( (:SSL_peek_ex, libssl), Cint, - (SSL, Ptr{UInt8}, Cint, Ptr{Csize_t}), + (SSL, Ptr{UInt8}, Csize_t, Ptr{Csize_t}), ssl.ssl, - peekbuf, + ssl.peekbuf, 1, - peekbytes + ssl.peekbytes ) if pending === nothing && err === nothing if ret == SSL_ERROR_NONE @@ -899,71 +1583,571 @@ function Base.eof(ssl::SSLStream)::Bool # to be processed, but not a full record, so we need to wait # for additional bytes to come in before we can process. If the # socket is at EOF they never will: the peer went away mid-record, - # so this is the end of the stream. Looping instead would spin, + # so this is the end of the stream. Looping instead would spin: # `haspending` stays true for the partial record and `eof(ssl.io)` # returns at once, without ever yielding. - eof(ssl.io) && return true + socketeof(ssl) && return true end continue end (ret, pending, err) end - # processing the record produced something for the peer (a KeyUpdate reply, or - # the alert of a record that failed): send it, or fail, without holding - # `eoflock`. A writer parked on a peer that is not reading would otherwise hold - # every other reader up through this lock. Then go round again: whether the + # processing the record produced something for the peer (a KeyUpdate reply; the + # alert of a record that failed goes with the abort): send it, or fail, without + # holding `eoflock`. A writer parked on a peer that is not reading would otherwise + # hold every other reader up through this lock. Then go round again: whether the # peek made bytes available is re-checked at the top. - finish_sslcall!(ssl, ret, pending, err) + finish_sslcall!(ssl, ret, pending, err; detach=true) end bytesavailable(ssl) > 0 && return false return !isopen(ssl) end """ - Close SSL stream. + close(ssl::SSLStream, shutdown::Bool=true) + +Closes the stream and its socket; returns `nothing`, possibly before all of it is done: +an abort sends what was produced before it (bar anything after a record already lost, +the records of a call cut short before it took them, and a close_notify a graceful close +cut short produced but never took), and the socket is closed, in the background; a +second close of the same kind returns while the first is under way, while an abort +during a graceful close takes over from it (see below). With `shutdown`, gracefully: +writes issued on other tasks before the close finish first, the stream staying readable +while they do, and one issued after it fails, as `iswritable` tells from the start of +the close; then the peer is sent the close_notify, and the socket is closed with what is +queued on it flushed. That can wait behind a writer parked on a peer that is not +reading, for as long as the peer keeps taking bytes; once nothing has moved for +`CLOSE_GRACE` seconds the parked write is failed and the close finishes as an abort. A +stream that has lost a record, one that failed to reach the peer, likewise ends aborted, +with no close_notify: none can follow the gap. A second graceful close, while the first +is under way or after it, returns at once. + +`close(ssl, false)` aborts. The stream is marked closed at once, and nothing produced +from then on is sent: a write under way fails at its next chunk, a new one at once. +What was produced before still goes out, in order (up to a record lost already, which +refuses all after it), and then the socket is closed. A +writer parked on a peer that is not reading holds that up until `ABORT_GRACE` seconds +pass with nothing moving; then the socket is closed outright and the parked write +fails. A graceful close still waiting for writes issued before it stops waiting at +once and returns; one whose close_notify was already taken sends it first, being ahead +of the abort. """ function Base.close(ssl::SSLStream, shutdown::Bool=true) - pending = Base.@lock ssl.lock begin - ssl.closed && return - closelocked!(ssl, shutdown) + if shutdown + closegracefully!(ssl) + else + # abort: tear the transport down whether or not the stream was closed before, + # a graceful close waiting behind a parked writer included. The wait for the lock + # held against interrupts too: this is how a cancelled task cleans up + abortsurely!(ssl) end - closesocket!(ssl, pending) + return +end + +# aborts the stream, as `close(ssl, false)` does; every cleanup that aborts a stream goes +# through this. With an interrupt held back, and `ssl.lock` taken surely: should an +# exception be thrown into the task while it waits for it, the abort and what follows it +# are left to a task of the library's, and the exception goes on (see `surely`). The +# abort failing is logged, not thrown, a cleanup's own error being the one to report +function abortsurely!(ssl::SSLStream) + holdinterrupts() do + surely(ssl.lock; then=r -> r === nothing || abortfailed!(ssl, r...)) do + try + abortlocked!(ssl) + nothing + catch ex + # whether the stream got marked closed read here, under the lock, rather + # than in a wait for it after + (ex, catch_backtrace(), ssl.closed) + end + end + end + return nothing +end + +# after an abort that failed, outside `ssl.lock`: if it got as far as marking the stream +# closed, the socket goes all the same, as a watch's does: a stream that reads as closed +# does not keep its socket open. If not (only a failure setting up its hold, the stream +# being marked closed first), the stream reads as open, and the socket is left to it. +# Said first, the cut saying itself should it fail +function abortfailed!(ssl::SSLStream, failure, bt, closed::Bool) + what = closed ? "cutting its socket" : "the stream is left open" + @guarded @error "OpenSSL: aborting a stream failed; $what" exception=(failure, bt) + closed && cut!(getfield(ssl, :data), nothing, failure) + return nothing end -# marks the stream closed and frees the SSL object; must run under `ssl.lock`, which -# the caller keeps holding. Returns what OpenSSL left in the write BIO, the close_notify -# when `shutdown` is set or the fatal alert of the call that failed, for `closesocket!` -# to send once the lock is released. -function closelocked!(ssl::SSLStream, shutdown::Bool) - if !ssl.closed +# marks the stream closed and aborts it, in one go: under `ssl.lock`, which the caller +# holds, and with an interrupt held back, so that nothing stops in between. Nothing in +# here waits but, on a failure, the giving up of a ticket and the cut (see `surely`), +# where a task injected with an exception could still land. Whatever fails on the way, +# the stream ends up marked closed (first, before anything that allocates) and claimed +# (in a `finally`), and either its abort's task set going or, should that task not have +# been made, its socket cut outright here, a parked write with it: every caller gets the +# same, and a stream is only ever marked closed without an abort by a graceful close. +# Failures are logged, last and one by one, so that a logger that throws stops none of +# this; nor is what it throws taken for the abort's. A second abort has nothing to add +function abortlocked!(ssl::SSLStream) + holdinterrupts() do + data = getfield(ssl, :data) + data.aborted && return + wasopen = !ssl.closed ssl.closed = true - if shutdown + closefailure = nothing + taskfailure = nothing + taken = nothing + task = nothing + try try - ssl_disconnect(ssl.ssl) - catch err - @debug "SSL disconnect failed" err + # the rest of the close (see `closelocked!`), first, so that nothing after + # can keep it from being done: the SSL object freed, and the alert, if + # there is one, taken with the stream marked closed, so no ticket can + # follow it + ticket = data.nextticket + try + # on a stream closed already, by a graceful close cut short on its + # way perhaps, the rest of the close done all the same (a second + # free does nothing) + # whatever a close cut short left in the buffer is not sent: with + # it emptied, the close hands out nothing + wasopen || empty!(data.buf) + taken = closerest!(ssl, false) + catch ex + # the alert given up, its ticket too should it have been handed out + # (`closerest!` frees the SSL object whatever it does) + empty!(data.buf) + data.nextticket == ticket || abandon!(data, ticket) + closefailure = (ex, catch_backtrace()) + end + try + task = abortertask(ssl, taken) + catch ex + taskfailure = (ex, catch_backtrace()) + end + finally + # claimed, whatever went wrong above: the task set going, or else the + # alert's ticket given up and the socket cut, the cut whatever the giving + # up does + data.aborted = true + if task === nothing + try + taken === nothing || abandon!(data, taken.ticket) + finally + cause = taskfailure === nothing ? nothing : taskfailure[1] + cut!(data, nothing, cause) + end + else + schedule(task) + end + end + finally + # whatever the above threw + logabort("the rest of its close failed; its alert is given up", closefailure) + logabort("making its task failed; its socket is cut", taskfailure) + end + end + return nothing +end + +# logs what went wrong in an abort, if anything did (see `@guarded`) +function logabort(what, failure) + failure === nothing && return + @guarded @error "OpenSSL: aborting a stream: $what" exception=failure + return +end + + +# the graceful close's: marks the stream closed, produces the close_notify and frees the +# SSL object; must run under `ssl.lock`, which the caller keeps holding. Returns the +# close_notify for `closegracefully!` to send (nothing should a record have been lost +# already, see `closerest!`). On a stream closed already, nothing: no ticket is handed +# out once the stream is marked closed, which is what lets the abort send everything +# ticketed before it and refuse nothing +function closelocked!(ssl::SSLStream) + ssl.closed && return nothing + ssl.closed = true + return closerest!(ssl, true) +end + +# the rest of a close, once the stream is marked closed: the graceful close's through +# `closelocked!`, the abort's (with no close_notify) through `abortlocked!`. Returns +# what OpenSSL left in the write BIO, the close_notify or the alert of the call that +# failed. Freeing the SSL object twice would do nothing the second time (`free` clears +# the pointer) +function closerest!(ssl::SSLStream, shutdown::Bool) + # the SSL object freed whatever the steps before it did: a graceful close waiting + # for writes stops waiting for them, then the close_notify is produced. Not when + # that step did not go through, nor when a record was lost already, on another task + # whose abort has yet to begin (a read's reply sent from a task of its own, a + # handshake round, a write whose abort was left to a task of the library's): the + # close_notify would be refused, the peer getting none, and `SSL_shutdown` would + # still mark the session as shut down cleanly, which keeps it resumable + notified = false + try + notify(ssl.closecond) + notified = true + finally + try + if shutdown && notified + # the check and `SSL_shutdown` in one section, so that no loss recorded + # before the close_notify exists goes unseen (`SSL_shutdown` only buffers + # what it produces, the write BIO callback taking no lock); one recorded + # after still refuses it, a close_notify then produced in good faith. A + # plain wait for `data.cond`, not `surely`: an exception landing in it + # costs the close_notify, and the close aborts + data = getfield(ssl, :data) + Base.@lock data.cond begin + data.lostfrom == typemax(Int) && ssl_disconnect(ssl.ssl) + end end + finally + free(ssl.ssl) end - free(ssl.ssl) end return take!(getfield(ssl, :data)) end -# sends what `closelocked!` returned, best effort, then closes the socket. Must be -# called without `ssl.lock` held: the peer may not be reading. -function closesocket!(ssl::SSLStream, pending) +# how long, without any progress, an abort lets the records still to go out, its alert +# included, and the normal close of the socket take before the socket handle is closed +# under them. Progress resets it: a peer that reads, however slowly, is not cut off, so +# this bounds a stall, not the whole close. The same for a graceful close, which should +# ride out a peer that merely pauses: a stalled peer costs the close_notify, and the +# peer a truncation error. Both settable, for tests that would rather not wait +const ABORT_GRACE = Ref(1.0) +const CLOSE_GRACE = Ref(10.0) + +# arms an `AbortWatch` over the socket, ticking every `grace` seconds +armwatch(ssl::SSLStream, grace::Real) = + (watch = AbortWatch(ssl); ticker(t -> watch(t), grace; interval=grace)) + +# the graceful close. Writes issued before it finish first, as they did when one +# `SSL_write_ex` under the lock wrote the whole of each: the close waits until none is +# in flight before it marks the stream closed. Then the close_notify takes its turn +# behind the records before it, and the socket is closed the normal way, which flushes +# what is queued on it and sends a FIN. A writer parked on a peer that is not reading +# would hold either wait for good, so a watch bounds the stall: once nothing has moved +# for `CLOSE_GRACE` it fails the parked write; the close_notify is then refused for the +# gap, or never produced, and the close turns into an abort. An abort meanwhile ends the +# wait. A second graceful close finds the first under way or done and does nothing more. +# Must be called without `ssl.lock` held. +function closegracefully!(ssl::SSLStream) + io = ssl.io + began = false + timer = nothing + sent = false try - drain!(getfield(ssl, :data), pending) + # inside the `try`: once the close has begun, the stream must end up closed + Base.@lock ssl.lock begin + # the bit set atomically: a write counting in at the same moment either + # gets in first and is waited for, or sees the bit and is refused + began = !ssl.closed && Threads.atomic_or!(ssl.wstate, CLOSING) & CLOSING == 0 + end + began || return + # the watch, should it give up, marks the stream closed, which ends the wait + timer = armwatch(ssl, CLOSE_GRACE[]) + pending = Base.@lock ssl.lock begin + # until no write issued before the close is in flight, or the stream is + # closed: by an abort, or by the watch giving up + while writesinflight(ssl) > 0 && !ssl.closed + wait(ssl.closecond) + end + # aborted meanwhile: then nothing comes back + closelocked!(ssl) + end + if pending !== nothing + # on failure this aborts the stream itself + drain!(ssl, pending) + sent = true + end catch err # the peer being gone is expected here; an interrupt or a cancellation of the # task is not ours to swallow err isa Union{Base.IOError, EOFError} || rethrow() - @debug "SSL alert not sent" err + @guarded @debug "SSL close_notify not sent" err + finally + # with an interrupt held back: a close interrupted while it waited still has to + # end the stream, and the abort takes locks, cancellation points where there are + # such + began && holdinterrupts() do + # no close_notify, from a shutdown that produced none, one that did not go + # out, or an abort meanwhile: the abort is the close that copes with a parked + # writer + finish = function () + timer === nothing || close(timer) + if sent + background(() -> through(() -> closequietly(io))) + else + close(ssl, false) + end + end + # handed off first, under `ssl.lock`: the watch's giving up checks it there, + # and a tick that raced the timer's close stands down. The lock taken surely, + # the hand-off and the rest of the close left to a task of the library's + # should an exception be thrown into this one meanwhile (see `surely`) + if sent + data = getfield(ssl, :data) + surely(() -> (data.handedoff = true), ssl.lock; then=_ -> finish()) + else + finish() + end + end + end + return +end + +# the abort's task, for `abortlocked!` to set going once it has marked the stream +# closed, `alert` being the alert of the call that failed, or nothing. Nothing +# produced after that point is sent any more (no call makes records nor takes a ticket +# once the stream is marked closed). What was produced before it is protocol-wise fine +# to send, so it goes out in order, the alert last, and once every ticket is through the +# socket is closed the normal way. A writer parked on a peer that is not reading would +# hold that up for good, so an `AbortWatch` closes the socket handle outright once +# `ABORT_GRACE` passes without progress. With nothing to send and nothing in flight, the +# socket is simply closed. +function abortertask(ssl::SSLStream, alert::Union{Nothing, PendingWrite}) + data = getfield(ssl, :data) + io = ssl.io + # every wait through a Ctrl-C landing on this task (see `runwhole`, `through`); the + # alert's socket write is no wait to hold one off across, having no bound: one landing + # there gives the alert up, cutting the socket, as for any write cut short + return unscheduled() do + timer = nothing + try + # nothing to send and nothing in flight: the socket is simply closed + idle = alert === nothing && + through(() -> Base.@lock(data.cond, drained(data))) + if !idle + timer = runwhole(() -> armwatch(ssl, ABORT_GRACE[])) + try + drain!(data, alert) + catch ex + # the drain has passed its ticket on whatever ended it + if ex isa InterruptException + lostinterrupt() + else + @guarded @debug "SSL alert not sent" ex + end + end + # the records ticketed before the abort are still going out; closing the + # socket now would take its write side from under them. The watch bounds + # this wait: once it has cut the socket, a ticket that never comes + # through (its task stopped before draining it) holds it no more + through() do + Base.@lock data.cond begin + while !drained(data) && !data.cut + wait(data.cond) + end + end + end + end + through(() -> closequietly(io)) + catch ex + # the watch not armed, or anything else gone wrong: the socket goes all the + # same, outright, a parked write with it; said after + try + cut!(data, nothing, ex) + finally + @guarded @error "OpenSSL: the abort of a stream failed; its socket is cut" exception=caught(ex) + end + finally + # `close` waits for the handle to be closed on every supported Julia, so the + # watch has nothing left to do; keeping it for another period would make a + # process that is exiting wait for it. Unless the close threw: then the + # watch stays on + timer === nothing || !socketclosed(io) || close(timer) + end + end +end + +# closes the socket, quietly about the peer being gone; and not at all once the handle +# is gone, which `close` would refuse with an error: the socket's finalizer may have run +# before the stream's +function closequietly(io::TCPSocket) + socketclosed(io) && return nothing + try + Base.close(io) + catch e + e isa Base.IOError || rethrow() + end + return nothing +end + +# the handle and status of a socket are the event loop's, to be read under its lock +function withiolock(f) + Base.iolock_begin() + try + return f() + finally + Base.iolock_end() + end +end + +socketclosed(io::TCPSocket) = withiolock(() -> handlegone(io)) + +# under `iolock`. A closed socket's handle is freed but, from Julia 1.9 on, not nulled: +# the status is what says whether the handle may still be touched +handlegone(io::TCPSocket) = io.handle == C_NULL || io.status == Base.StatusClosed + +# the watchdog of an abort or a close: called every grace period, it closes the socket +# handle outright when nothing has moved since the last time. A record going out moves the +# turn; bytes of a record under way leaving for the kernel shrink libuv's write queue, +# so a peer that reads slowly is not cut off. On Windows libuv only takes a write off +# the queue once it is complete, so there a slowly read record is cut off after one +# period with no turn passed. Stops itself once the socket is closed. +mutable struct AbortWatch + # the stream: when the watch gives up it marks it closed as well, which ends a + # graceful close's wait for the writes in flight; for an abort's watch it is closed + # already, and that does nothing + stream::SSLStream + turn::Int + queued::Csize_t +end +AbortWatch(ssl::SSLStream) = AbortWatch(ssl, watchstate(ssl)...) + +# the turn and libuv's write queue: what moves when bytes go out +function watchstate(ssl::SSLStream) + data = getfield(ssl, :data) + return Base.@lock(data.cond, data.turn), writequeuesize(getfield(ssl, :io)) +end + +function (watch::AbortWatch)(timer::Timer) + stream = watch.stream + data = getfield(stream, :data) + # the whole decision under `ssl.lock`, where a graceful close hands off: a close that + # got its close_notify out and handed the socket to its normal close is done with its + # watch, and one whose close_notify went out after this last sampled has moved. Else + # the socket closed already, by someone else, or nothing moved: either way no write + # in flight can finish now, and the watch gives up + verdict = :moved + cause = nothing + try + Base.@lock stream.lock begin + if data.handedoff + verdict = :stand + else + moved = false + if !socketclosed(getfield(stream, :io)) + turn, queued = watchstate(stream) + moved = (turn, queued) != (watch.turn, watch.queued) + watch.turn, watch.queued = turn, queued + end + if !moved + # the verdict stands whatever the abort does + verdict = :gaveup + abortlocked!(stream) + end + end + end + catch ex + # failed before a verdict: a watch that cannot judge progress gives up, rather + # than leave the stall it bounds unbounded. Not for an interrupt, which is not + # the watch's to judge by: that tick is lost, as in any timer, and the next one + # judges + if !(ex isa InterruptException) + verdict === :moved && (verdict = :gaveup) + cause = ex + end + rethrow() finally - @async try - Base.close(ssl.io) - catch e - e isa Base.IOError || rethrow() + if verdict === :gaveup + # the socket goes, which fails a parked write, and the watch stops; whatever + # went wrong above, which the ticker then hears of, and which the cut names + # should it fail too + try cut!(data, nothing, cause) finally close(timer) end + elseif verdict === :stand + close(timer) + end + end + return +end + +# bytes handed to libuv for the socket that it has not passed to the kernel yet. Zero +# when the socket is gone, or should libuv not answer (it has since 1.19; Julia 1.6 +# ships 1.42): the watch then goes by the turn alone. +function writequeuesize(io::TCPSocket) + withiolock() do + handlegone(io) && return Csize_t(0) + try + return ccall(:uv_stream_get_write_queue_size, Csize_t, (Ptr{Cvoid},), io.handle) + catch + return Csize_t(0) + end + end +end + +# set only by tests, to take the fallback below as if the internals were gone +const FORCECLOSE_FALLBACK = Ref(false) + +# chunks kept alive for writes libuv may still hold, see `forceclose!` +const KEPT = Base.IdSet{Any}() +const KEPT_LOCK = ReentrantLock() + +# set only by tests: called with the socket and the chunk whenever a chunk is kept +const ONKEEP = Ref{Any}(nothing) + +# closes the socket handle outright: the writes queued on it fail with `ECANCELED` +# instead of being flushed first, as `close(::TCPSocket)` would do. What the kernel +# already holds it goes on delivering, unless it also holds inbound bytes never read, +# in which case (on Linux at least) it resets the connection and drops it all: the +# peer then sees a reset where records were counted as sent. A handle `uv_close` was +# already called on must not get a second call, a pending shutdown from `close` is no +# obstacle. This is what `close` itself does with a socket that never connected, and it +# is built on the same internals (checked on Julia 1.10, 1.12 and the 1.14 nightly); +# should they be gone, fall back to the normal close, which then may wait behind a +# parked write. Should they change in meaning instead, `CancelledCloser` and +# `AbortDuringGracefulClose` fail: a parked write is not failed any more. +# +# `keep` is the chunk of a write cancelled while under way, which libuv still holds a +# pointer into and nothing else keeps alive any more. Closing the handle outright +# cancels that write, but not necessarily at once (on Windows an overlapped send is +# cancelled when its completion comes in); the normal close of the fallback does not +# cancel it at all. Either way the chunk is kept referenced until the socket is closed +function forceclose!(io::TCPSocket, keep=nothing) + outright = false + try + FORCECLOSE_FALLBACK[] && error("the fallback, taken on purpose") + withiolock() do + if !handlegone(io) && ccall(:uv_is_closing, Cint, (Ptr{Cvoid},), io.handle) == 0 + ccall(:jl_forceclose_uv, Cvoid, (Ptr{Cvoid},), io.handle) + io.status = Base.StatusClosing + end + end + outright = true + catch err + @guarded @debug "could not close the socket handle outright" err + end + (outright && keep === nothing) && return + background() do + if keep !== nothing + # the chunk kept, by the task that holds it till then, one of the library's + # that nothing can throw into but a Ctrl-C, whole (see `runwhole`) + runwhole(() -> Base.@lock(KEPT_LOCK, push!(KEPT, keep))) + # not in a hold: the hook is the tests' code, and catches its own + onkeep(io, keep) + end + # the normal close: after the outright one it waits for the handle to be + # released, otherwise it is the close; through interrupts (see `through`). The + # chunk is let go only once the socket is closed, whole, and else stays held: a + # leak, not a pointer into freed memory + through(() -> closequietly(io)) + keep === nothing || + runwhole(() -> socketclosed(io) && Base.@lock(KEPT_LOCK, delete!(KEPT, keep))) + end + return +end + +# the tests' hook, told once a chunk is kept, on the task that keeps it; nothing it +# throws gets in the way. Set after that task began, as a rule, so newer than its world +function onkeep(io, keep) + hook = ONKEEP[] + hook === nothing && return + try + Base.invokelatest(hook, io, keep) + catch ex + if ex isa InterruptException + lostinterrupt() + else + @guarded @error "OpenSSL: the ONKEEP hook failed" exception=caught(ex) end end return diff --git a/test/runtests.jl b/test/runtests.jl index 9cf2885..f2df64d 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -119,21 +119,24 @@ end end end @test x509_cert !== nothing - - @test occursin("/C=", String(x509_cert.subject_name)) - @test occursin("/OU=", String(x509_cert.subject_name)) - @test occursin("/CN=", String(x509_cert.subject_name)) - - # the roots in the bundle are self signed, so the issuer carries the same fields - @test occursin("/C=", String(x509_cert.issuer_name)) - @test occursin("/OU=", String(x509_cert.issuer_name)) - @test occursin("/CN=", String(x509_cert.issuer_name)) - - s_before_time = replace(String(x509_cert.time_not_before), r" +" => " ") - @test DateTime(s_before_time, dateformat"u d HH:MM:SS yyyy Z") < today() - - s_after_time = replace(String(x509_cert.time_not_after), r" +" => " ") - @test DateTime(s_after_time, dateformat"u d HH:MM:SS yyyy Z") > today() + # the rest only with a certificate to look at, a failure rather than an error + # otherwise + if x509_cert !== nothing + @test occursin("/C=", String(x509_cert.subject_name)) + @test occursin("/OU=", String(x509_cert.subject_name)) + @test occursin("/CN=", String(x509_cert.subject_name)) + + # the roots in the bundle are self signed, so the issuer carries the same fields + @test occursin("/C=", String(x509_cert.issuer_name)) + @test occursin("/OU=", String(x509_cert.issuer_name)) + @test occursin("/CN=", String(x509_cert.issuer_name)) + + s_before_time = replace(String(x509_cert.time_not_before), r" +" => " ") + @test DateTime(s_before_time, dateformat"u d HH:MM:SS yyyy Z") < today() + + s_after_time = replace(String(x509_cert.time_not_after), r" +" => " ") + @test DateTime(s_after_time, dateformat"u d HH:MM:SS yyyy Z") > today() + end # finalizer will cleanup #finalize(x509_cert) @@ -644,10 +647,8 @@ end end -@testset "ConcurrentReadWrite" begin - # A write that is waiting for the peer must not stop reads on the same stream: - # `SSL_write_ex` and `SSL_read_ex` share `ssl.lock`, so the socket write the write - # BIO does has to happen outside it. +# a server context with a self-signed certificate for localhost +function selfsigned_server_ctx() cert = X509Certificate() key = EvpPKey(rsa_generate_key()) cert.public_key = key @@ -658,55 +659,172 @@ end 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 - server_ctx = OpenSSL.SSLContext(OpenSSL.TLSServerMethod(), "") - OpenSSL.ssl_use_certificate(server_ctx, cert) - OpenSSL.ssl_use_private_key(server_ctx, key) - - port, server = Sockets.listenany(ip"127.0.0.1", 20000) - payload = collect(0x01:0x40) - stop = Channel{Nothing}(1) +# a connected client and the task running the server side of the handshake, which +# returns the server's `SSLStream` +function connected_pair(server_ctx, server) server_task = @async begin - sock = accept(server) - ssl = OpenSSL.SSLStream(server_ctx, sock) - while true + ssl = OpenSSL.SSLStream(server_ctx, accept(server)) + Sockets.accept(ssl) + ssl + end + port = Sockets.getsockname(server)[2] + client = OpenSSL.SSLStream(OpenSSL.SSLContext(OpenSSL.TLSClientMethod(), ""), + Sockets.connect(ip"127.0.0.1", port)) + Sockets.connect(client; require_ssl_verification=false) + return client, fetch(server_task) +end + +# runs `f(kept)` with the chunks kept for `client`'s socket collected into `kept`, a +# vector under `lock`; whatever other tasks keep meanwhile is left out +function withkept(f, client) + kept = Any[] + keptlock = ReentrantLock() + OpenSSL.ONKEEP[] = (io, chunk) -> io === client.io && Base.@lock(keptlock, push!(kept, chunk)) + try + return f(() -> Base.@lock(keptlock, copy(kept))) + finally + OpenSSL.ONKEEP[] = nothing + end +end + +# waits for `task`, a read of the server stream `ssl`, under a deadline: its outcome +# (see `taskoutcome`), or `:stalled` should it not finish, both streams then aborted and +# the client's socket cut, each whatever the one before it throws, so that the read is +# released and nothing hangs the suite. The verdict is the caller's to assert, so that a +# failure names the call +function awaitread(task, ssl, client; timeout=30.0, onstall=nothing) + timedwait(() -> istaskdone(task), timeout) === :ok && return taskoutcome(task) + try + # what the caller would know of the stall before the abort changes it + onstall === nothing || onstall() + finally + try + close(ssl, false) + finally try - Sockets.accept(ssl) - break - catch - eof(sock) && return - sleep(0.01) + close(client, false) + finally + OpenSSL.cut!(client.data) end end - write(ssl, payload) - # and from here on the server reads nothing, so the client's write parks - take!(stop) - close(ssl) end + return :stalled +end - client = OpenSSL.SSLStream(OpenSSL.SSLContext(OpenSSL.TLSClientMethod(), ""), - Sockets.connect(ip"127.0.0.1", port)) - Sockets.connect(client; require_ssl_verification=false) +# a finished task's value, or, should it have failed, the exception it failed with (not +# the TaskFailedException `fetch` wraps it in), for the caller to assert on rather than +# to be thrown out of the testset by +taskoutcome(task) = istaskfailed(task) ? task.result : fetch(task) + +# `task`'s outcome (see `taskoutcome`) should it finish within `timeout`, else +# `placeholder`: the verdict is the caller's to assert, and nothing waits on a task that +# stalled +boundedfetch(task, placeholder; timeout=30.0) = + timedwait(() -> istaskdone(task), timeout) === :ok ? taskoutcome(task) : placeholder + +# `f`, a read of the server stream `ssl` catching its own errors, on a task of its own, +# waited for as `awaitread` does +boundedread(f, ssl, client; kwargs...) = awaitread(@async(f()), ssl, client; kwargs...) + +# for a test whose `client` the caller has closed and whose `reader` reads `ssl`: waits +# for the read as `awaitread` does, runs `check` on its outcome should it have finished, +# and closes `ssl` gracefully whatever `check` or the read's own failure throws. The +# client's socket is checked to have closed of itself either way: after the check, or, +# should the read stall, before the abort closes it. It is cut last, which is bounded (a +# normal close could wait for a close that never comes), so that nothing of the pair +# outlasts the testset +function awaitpeer(check, reader, ssl, client) + closedofitself() = @test timedwait(() -> !isopen(client.io), 10.0) === :ok + stalled = false + try + r = awaitread(reader, ssl, client; onstall=closedofitself) + stalled = r === :stalled + @test !stalled + if !stalled + try + check(r) + finally + closedofitself() + end + end + finally + try + # aborted already, should the read have stalled + stalled || close(ssl) + finally + OpenSSL.cut!(client.data) + end + end + return +end - # keep writing until the peer's receive window is full and the writer parks: how - # much that takes depends on the platform's socket buffers - stop_writing = Threads.Atomic{Bool}(false) +# writes until the peer's receive window is full and the writer parks on the socket; +# returns the task and the count of completed writes. Each write is more than what goes +# to OpenSSL in one call, so that the parked one has a chunk still to go +function park_writer(client, stop_writing) written = Threads.Atomic{Int}(0) - chunk = zeros(UInt8, 1024 * 1024) + chunk = zeros(UInt8, OpenSSL.SSL_WRITE_CHUNK + OpenSSL.SSL_WRITE_CHUNK รท 2) writer = @async try while !stop_writing[] write(client, chunk) Threads.atomic_add!(written, length(chunk)) end + nothing catch ex ex end + # parked means: bytes handed to libuv that have not left for the kernel, the same + # amount half a second later, and no write completed meanwhile. A writer that is + # merely slow, mid-encryption say, has nothing queued in libuv + queued() = OpenSSL.writequeuesize(client.io) parked = timedwait(60.0; pollint=0.5) do - before = written[] + before = (written[], queued()) sleep(0.5) - !istaskdone(writer) && written[] == before + !istaskdone(writer) && before[2] > 0 && (written[], queued()) == before end - @test parked === :ok + # nothing that follows makes sense without a parked writer, and cancelling one that + # is running is not allowed: stop here rather than fail all down the line + if parked !== :ok + outcome = istaskdone(writer) ? fetch(writer) : "still writing" + close(client, false) + error("the writer did not park on the socket: $outcome") + end + return writer, written +end + +# whether every run of one writer's byte in `received` is whole writes of that writer long +function wholewrites(received, chunklen) + i = 1 + while i <= length(received) + j = i + while j <= length(received) && received[j] == received[i] + j += 1 + end + (j - i) % chunklen(Int(received[i])) == 0 || return false + i = j + end + return true +end + +@testset "ConcurrentReadWrite" begin + # A write that is waiting for the peer must not stop reads on the same stream: + # `SSL_write_ex` and `SSL_read_ex` share `ssl.lock`, so the socket write the write + # BIO does has to happen outside it. + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + client, ssl = connected_pair(server_ctx, server) + payload = collect(0x01:0x40) + write(ssl, payload) + # and from here on the server reads nothing, so the client's write parks + + stop_writing = Threads.Atomic{Bool}(false) + writer, _ = park_writer(client, stop_writing) # the write waits for the socket without holding the lock reads need @test timedwait(() -> !islocked(client.lock), 5.0) === :ok @@ -719,58 +837,38 @@ end @test timedwait(() -> istaskdone(reader), 30.0) === :ok @test buff == payload - # the server has to go away first: the client still has 8 MB parked against a peer - # that is not reading, so its own close cannot finish until that write fails + # the server has to go away first: the client still has megabytes parked against a + # peer that is not reading, so its own close cannot finish until that write fails stop_writing[] = true - put!(stop, nothing) - @test timedwait(() -> istaskdone(server_task), 30.0) === :ok + close(ssl) close(client) close(server) @test timedwait(() -> istaskdone(writer), 30.0) === :ok end @testset "ConcurrentWriters" begin - # Two tasks writing to one stream: each SSL call takes its own ciphertext under + # Several tasks writing to one stream: each SSL call takes its own ciphertext under # `ssl.lock` and the socket writes go out in that order, so the records arrive in # the order OpenSSL made them (out of order would fail the record MAC on the peer) # and neither writer returns before its own bytes reached the socket. - 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) - - server_ctx = OpenSSL.SSLContext(OpenSSL.TLSServerMethod(), "") - OpenSSL.ssl_use_certificate(server_ctx, cert) - OpenSSL.ssl_use_private_key(server_ctx, key) - - # different sizes per writer: the byte count `SSL_write_ex` reports has to be the - # one of this task's call, not of whichever task ran last on the stream + # Different sizes per writer: the byte count `SSL_write_ex` reports has to be the + # one of this task's call, not of whichever task ran last on the stream. And all + # larger than what goes to OpenSSL in one call: a write still arrives in one piece. nwriters = 4 - nchunks = 64 - chunklen(id) = id * 32 * 1024 + nchunks = 4 + chunklen(id) = OpenSSL.SSL_WRITE_CHUNK + id * (OpenSSL.SSL_WRITE_CHUNK รท 2) total = sum(nchunks * chunklen(id) for id in 1:nwriters) + server_ctx = selfsigned_server_ctx() port, server = Sockets.listenany(ip"127.0.0.1", 20000) + client, ssl = connected_pair(server_ctx, server) server_task = @async begin - sock = accept(server) - ssl = OpenSSL.SSLStream(server_ctx, sock) - Sockets.accept(ssl) received = Vector{UInt8}(undef, total) read!(ssl, received) close(ssl) received end - client = OpenSSL.SSLStream(OpenSSL.SSLContext(OpenSSL.TLSClientMethod(), ""), - Sockets.connect(ip"127.0.0.1", port)) - Sockets.connect(client; require_ssl_verification=false) - writers = map(1:nwriters) do id @async begin chunk = fill(UInt8(id), chunklen(id)) @@ -781,156 +879,1247 @@ end end @test timedwait(() -> all(istaskdone, writers), 60.0) === :ok @test !any(istaskfailed, writers) - @test timedwait(() -> istaskdone(server_task), 60.0) === :ok - @test !istaskfailed(server_task) - if !istaskfailed(server_task) - received = fetch(server_task) + received = boundedfetch(server_task, :stalled; timeout=60.0) + @test received isa Vector{UInt8} + # a read that stalled or failed released, rather than left to outlast the testset + received isa Vector{UInt8} || close(ssl, false) + if received isa Vector{UInt8} @test length(received) == total # every writer's bytes all arrived, whatever the interleaving for id in 1:nwriters @test count(==(UInt8(id)), received) == nchunks * chunklen(id) end + # and each write in one piece: a run of one writer's byte is whole writes long + @test wholewrites(received, chunklen) end close(client) close(server) end -@testset "CancelledWriter" begin - # A writer cancelled while it waits for its turn on the socket must not leave a hole - # in the write order that parks every later writer and `close` forever. Its record - # never reaches the peer, so the stream is closed and later writes fail right away. - 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) +@testset "CancelledCloser" begin + # A close cancelled while it waits for a parked writer, issued before it, to be + # through must leave nothing hanging: the stream is aborted, which fails the parked + # write. A writer issued after the close is refused at once, not kept waiting. + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + client, ssl = connected_pair(server_ctx, server) + # the server reads nothing, so the client's writer parks on the socket - server_ctx = OpenSSL.SSLContext(OpenSSL.TLSServerMethod(), "") - OpenSSL.ssl_use_certificate(server_ctx, cert) - OpenSSL.ssl_use_private_key(server_ctx, key) + stop_writing = Threads.Atomic{Bool}(false) + parked_writer, _ = park_writer(client, stop_writing) + stop_writing[] = true + + closer = @async close(client) + third_writer = @async try + write(client, UInt8[2]) + catch ex + ex + end + # the closer has to be blocked before it is cancelled: delivering an exception to a + # task that is running is not allowed. Tasks made here run on this thread, so once + # this one runs again the closer is blocked, waiting for the parked writer + sleep(0.2) + @test !istaskdone(closer) + @test isopen(client) + # issued after the close began: refused already + @test istaskdone(third_writer) && fetch(third_writer) isa Base.IOError + schedule(closer, InterruptException(); error=true) + @test timedwait(() -> istaskdone(closer), 5.0) === :ok + @test istaskfailed(closer) + # the close was given up half way, so the stream is aborted: that fails the parked + # write, without the server having to do anything + @test !isopen(client) + @test timedwait(() -> istaskdone(parked_writer), 30.0) === :ok + @test istaskdone(parked_writer) && fetch(parked_writer) isa Exception + @test timedwait(() -> !isopen(client.io), 30.0) === :ok + close(ssl) + close(server) +end +# `closewrite` on a socket needs Julia 1.8 +if VERSION >= v"1.8" +@testset "TruncatedRecordEOF" begin + # A peer that goes away in the middle of a record leaves a partial record buffered: + # `haspending` stays true, `SSL_peek_ex` keeps asking for more bytes, and the socket + # is at EOF. `eof` has to report the end of the stream rather than loop on that. + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + client, ssl = connected_pair(server_ctx, server) + server_task = @async eof(ssl) + + # a record header announcing 100 bytes, followed by only 3 of them, then a FIN. Only + # the write side: closing the socket with the session tickets the server sent + # after the handshake still unread would send a RST instead + write(client.io, UInt8[0x17, 0x03, 0x03, 0x00, 0x64, 0xaa, 0xbb, 0xcc]) + closewrite(client.io) + + # the spin never yields, so on one thread a hang here would starve this task too; + # a task that does not finish is reported by the timeout, a spinning one by CI + @test timedwait(() -> istaskdone(server_task), 30.0) === :ok + @test istaskdone(server_task) && fetch(server_task) === true + # and, as `eof` on a socket, it does not close the stream: the peer may have shut + # down its side only, and a reply may still be due + @test isopen(ssl) + close(client) + close(ssl) + close(server) +end +end + +@testset "VerifyFailureCloses" begin + # a peer whose certificate does not verify: the error is what it was, the stream is + # closed rather than left usable + server_ctx = selfsigned_server_ctx() port, server = Sockets.listenany(ip"127.0.0.1", 20000) - stop = Channel{Nothing}(1) server_task = @async begin - sock = accept(server) - ssl = OpenSSL.SSLStream(server_ctx, sock) - Sockets.accept(ssl) - # the server reads nothing, so the client's writer parks on the socket, and it - # goes away first: a client socket closed with data still queued both ways is - # torn down differently from one platform to the next - take!(stop) + ssl = OpenSSL.SSLStream(server_ctx, accept(server)) + try + Sockets.accept(ssl) + catch + end close(ssl) end - - client = OpenSSL.SSLStream(OpenSSL.SSLContext(OpenSSL.TLSClientMethod(), ""), + client = OpenSSL.SSLStream(OpenSSL.SSLContext(OpenSSL.TLSClientMethod()), Sockets.connect(ip"127.0.0.1", port)) - Sockets.connect(client; require_ssl_verification=false) + @test_throws OpenSSL.OpenSSLError Sockets.connect(client) + @test !isopen(client) + @test timedwait(() -> istaskdone(server_task), 10.0) === :ok + close(server) +end - # fill the peer's receive window so the first writer parks on the socket write - stop_writing = Threads.Atomic{Bool}(false) - written = Threads.Atomic{Int}(0) - chunk = zeros(UInt8, 1024 * 1024) - parked_writer = @async try - while !stop_writing[] - write(client, chunk) - Threads.atomic_add!(written, length(chunk)) - end +@testset "SocketErrorCloses" begin + # a peer that resets the connection mid-record: the error from the socket ends the + # stream, so the next call does not run into it again + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + client, ssl = connected_pair(server_ctx, server) + server_task = @async try + eof(ssl) catch ex ex end - parked = timedwait(60.0; pollint=0.5) do - before = written[] - sleep(0.5) - !istaskdone(parked_writer) && written[] == before + # a partial record, then a close with the session tickets the server sent still + # unread, which makes the kernel reset the connection rather than end it + sleep(0.2) + write(client.io, UInt8[0x17, 0x03, 0x03, 0x00, 0x64, 0xaa, 0xbb, 0xcc]) + close(client.io) + outcome = awaitread(server_task, ssl, client) + @test outcome !== :stalled + if outcome !== :stalled + # on Linux a close with inbound bytes unread is a reset for sure; elsewhere it + # may be a FIN, which is the end of the stream and nothing more + Sys.islinux() && @test outcome isa Base.IOError + if outcome isa Base.IOError + @test !isopen(ssl) + # and the error is not run into again: the stream is simply at its end + @test eof(ssl) === true + else + @test outcome === true + end end - @test parked === :ok + close(client) + close(ssl) + close(server) +end - # a second writer queues behind it, waiting for its turn, and a third behind that - cancelled_writer = @async write(client, UInt8[1]) - third_writer = @async try - write(client, UInt8[2]) +@testset "CloseNotifyEOF" begin + # the peer's close_notify surfaces from `eof` as the "unexpected EOF" `IOError`, and + # closes the stream. The peek that sees it produces nothing to send, but the error + # path after the lock is released has to see the peek's result. + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + client, ssl = connected_pair(server_ctx, server) + close(client) + err = boundedread(ssl, client) do + try + eof(ssl) + nothing + catch ex + ex + end + end + @test err !== :stalled + if err !== :stalled + @test err isa Base.IOError + @test occursin("unexpected EOF", sprint(showerror, err)) + @test !isopen(ssl) + end + close(ssl) + close(server) +end + +@testset "CloseWithQueuedWriter" begin + # `close` started while a writer is parked and another waits behind it: both were + # issued first, so both finish, the parked one's remaining chunk included, and the + # close_notify follows their records: the peer reads them all and then the + # close_notify, not a gap or a cut write. + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + client, ssl = connected_pair(server_ctx, server) + + stop_writing = Threads.Atomic{Bool}(false) + parked_writer, written = park_writer(client, stop_writing) + stop_writing[] = true + queued_writer = @async try + write(client, UInt8[1]) catch ex ex end - # both have to be parked on the condition before one is cancelled: delivering an - # exception to a task that is still running (possible on another thread) is not - # allowed, and the cancellation has to land in the wait for the turn - data = client.data - queued() = Base.@lock data.cond data.waiting - @test timedwait(() -> queued() == 2, 5.0) === :ok - @test !istaskdone(cancelled_writer) - @test !istaskdone(third_writer) - # the second one gets cancelled while it waits for its turn - schedule(cancelled_writer, InterruptException(); error=true) - @test timedwait(() -> istaskdone(cancelled_writer), 5.0) === :ok - @test istaskfailed(cancelled_writer) - # the cancelled writer's record is lost, so the stream is unusable and closed - @test !isopen(client) + closer = @async close(client) + # the close waits for the writes issued before it, the parked one and the one behind + sleep(0.2) + @test !istaskdone(closer) + @test isopen(client) + + # now the server reads everything, up to the close_notify + outcome = boundedread(ssl, client) do + n = 0 + try + while !eof(ssl) + n += length(readavailable(ssl)) + end + (n, nothing) + catch ex + (n, ex) + end + end + @test outcome !== :stalled + # what the writers and the peer did, once the peer read it all (a stall aborted the + # stream, which the writers would then report) + if outcome !== :stalled + @test boundedfetch(parked_writer, :stalled) === nothing + # the waiting writer was issued before the close: its byte went out before the + # close_notify, whichever of the two got the writer lock first + @test boundedfetch(queued_writer, :stalled) == 1 + # the close returned, and did not throw + @test boundedfetch(closer, :stalled) === nothing + nread, err = outcome + @test nread == written[] + 1 + # the close_notify, not a MAC failure + @test err isa Base.IOError + @test occursin("unexpected EOF", sprint(showerror, err)) + end + close(ssl) + close(server) +end + +@testset "AbortDuringGracefulClose" begin + # `close(ssl, false)` has to take the transport down even when a graceful close is + # already waiting for its turn behind a writer parked on a peer that is not reading. + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + client, ssl = connected_pair(server_ctx, server) - # the server goes away, which fails the parked write; the writer queued behind the - # cancelled one then fails too, instead of waiting for a turn that never comes + stop_writing = Threads.Atomic{Bool}(false) + parked_writer, _ = park_writer(client, stop_writing) stop_writing[] = true - put!(stop, nothing) - @test timedwait(() -> istaskdone(server_task), 30.0) === :ok + queued_writer = @async write(client, UInt8[1]) + closer = @async close(client) + # both wait behind the parked write: the writer for the writer lock, the close for + # the writes issued before it to be through + sleep(0.2) + @test !istaskdone(closer) + @test isopen(client) + + # the server never reads, so only the abort can end these. Its watch waits longer + # here than the close is given below: the close has to stop waiting for the parked + # write at once, on being told the stream is closed, not once some watch has failed + # that write; the close's own ticks at `CLOSE_GRACE` + grace = OpenSSL.ABORT_GRACE[] + OpenSSL.ABORT_GRACE[] = 5.0 + prompt = try + close(client, false) + @test !isopen(client) + timedwait(() -> istaskdone(closer), 2.0) + finally + OpenSSL.ABORT_GRACE[] = grace + end + @test OpenSSL.CLOSE_GRACE[] >= 5 + @test prompt === :ok + @test timedwait(() -> istaskdone(parked_writer) && istaskdone(closer), 30.0) === :ok + @test istaskdone(parked_writer) && fetch(parked_writer) isa Exception + @test timedwait(() -> istaskdone(queued_writer), 30.0) === :ok + @test istaskfailed(queued_writer) + @test timedwait(() -> !isopen(client.io), 30.0) === :ok + close(ssl) + close(server) +end + +@testset "GracefulCloseBehindParkedWriter" begin + # `close(ssl)` waits for its close_notify's turn behind a writer parked on a peer + # that is not reading; it must not wait for good. The watch fails the parked write + # once nothing has moved for a period, and the close finishes as an abort. + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + client, ssl = connected_pair(server_ctx, server) + stop_writing = Threads.Atomic{Bool}(false) + parked_writer, _ = park_writer(client, stop_writing) + stop_writing[] = true + # the close waits out `CLOSE_GRACE` of no progress; not the full ten seconds here. + # Put back once the closer is through: it reads the grace when it gets to run + grace = OpenSSL.CLOSE_GRACE[] + OpenSSL.CLOSE_GRACE[] = 3.0 + closed = try + closer = @async close(client) + sleep(0.2) + # while it waits the stream refuses new writes, at once rather than behind the + # parked writer, and says why; it is still open + @test !iswritable(client) + @test isopen(client) + refused = @elapsed err = try + write(client, UInt8[1]) + nothing + catch ex + ex + end + @test err isa Base.IOError && occursin("being closed", sprint(showerror, err)) + @test refused < 1.0 + # and was never counted in: only the parked writer is in flight + @test OpenSSL.writesinflight(client) == 1 + # a second close while the first waits returns at once + second = @async close(client) + @test timedwait(() -> istaskdone(second), 1.0) === :ok + @test !istaskdone(closer) + timedwait(() -> istaskdone(closer), 30.0) + finally + OpenSSL.CLOSE_GRACE[] = grace + end + @test closed === :ok @test timedwait(() -> istaskdone(parked_writer), 30.0) === :ok - @test timedwait(() -> istaskdone(third_writer), 30.0) === :ok - @test istaskdone(third_writer) && fetch(third_writer) isa Base.IOError - # and neither does close + @test istaskdone(parked_writer) && fetch(parked_writer) isa Exception + @test timedwait(() -> !isopen(client.io), 30.0) === :ok + close(ssl) + close(server) +end + +@testset "CancelledInFlightWriter" begin + # A writer cancelled inside the socket write itself, not while waiting for its turn: + # libuv still holds the write, and a pointer into the chunk, so the socket has to be + # closed outright, at once; a normal close would wait behind that very write. + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + client, ssl = connected_pair(server_ctx, server) + stop_writing = Threads.Atomic{Bool}(false) + parked_writer, _ = park_writer(client, stop_writing) + stop_writing[] = true + schedule(parked_writer, InterruptException(); error=true) + @test timedwait(() -> istaskdone(parked_writer), 5.0) === :ok + @test istaskdone(parked_writer) && fetch(parked_writer) isa InterruptException + @test !isopen(client) + # the server never reads: only closing the handle outright can end this + @test timedwait(() -> !isopen(client.io), 5.0) === :ok closer = @async close(client) - @test timedwait(() -> istaskdone(closer), 30.0) === :ok + @test timedwait(() -> istaskdone(closer), 5.0) === :ok + close(ssl) + + # through the fallback, as without the internals `forceclose!` builds on: the + # normal close it falls back to does not cancel the write, which libuv still holds + # with a pointer into the chunk; the chunk stays referenced until that close is + # through, here once the peer has read what was queued + client, ssl = connected_pair(server_ctx, server) + stop_writing = Threads.Atomic{Bool}(false) + parked_writer, _ = park_writer(client, stop_writing) + stop_writing[] = true + # this write's chunk is told apart from anything else held (a chunk whose close + # never got through stays held for good) by what was held before + held(chunk) = Base.@lock(OpenSSL.KEPT_LOCK, chunk in OpenSSL.KEPT) + OpenSSL.FORCECLOSE_FALLBACK[] = true + try withkept(client) do kept + schedule(parked_writer, InterruptException(); error=true) + @test timedwait(() -> istaskdone(parked_writer), 5.0) === :ok + # kept, however soon it was let go again (on the nightly `close` does not wait + # behind the write) + @test timedwait(() -> length(kept()) == 1, 5.0) === :ok + # the peer reads, the queued write goes out, the close gets through; the read + # started whatever was kept, so that the close is not left behind the write + reader = @async try + while !eof(ssl.io) + readavailable(ssl.io) + end + catch + end + k = kept() + @test length(k) == 1 + if length(k) == 1 + @test timedwait(() -> !held(only(k)), 30.0) === :ok + end + @test timedwait(() -> istaskdone(reader), 30.0) === :ok + end + finally + # whatever failed above, the tests after this one close for real + OpenSSL.FORCECLOSE_FALLBACK[] = false + end + close(ssl) + + # cancelled by any exception, not only an interrupt: the writer sees it, and the rest + # happens all the same, the chunk kept, the cut recorded, the ticket through + client, ssl = connected_pair(server_ctx, server) + stop_writing = Threads.Atomic{Bool}(false) + parked_writer, _ = park_writer(client, stop_writing) + stop_writing[] = true + cancel = ErrorException("cancelled") + withkept(client) do kept + schedule(parked_writer, cancel; error=true) + @test timedwait(() -> istaskdone(parked_writer), 5.0) === :ok + @test istaskdone(parked_writer) && fetch(parked_writer) === cancel + @test timedwait(() -> length(kept()) == 1, 5.0) === :ok + end + data = client.data + @test Base.@lock(data.cond, data.cut && OpenSSL.drained(data)) + @test timedwait(() -> !isopen(client.io), 5.0) === :ok + close(client) + close(ssl) close(server) end -@testset "TruncatedRecordEOF" begin - # A peer that goes away in the middle of a record leaves a partial record buffered: - # `haspending` stays true, `SSL_peek_ex` keeps asking for more bytes, and the socket - # is at EOF. `eof` has to report the end of the stream rather than loop on that. - 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) +@testset "WriteCount" begin + # the closing bit and the count of writes in flight share one word; the bit is the + # sign bit, which exists whatever the width of `Int` + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + client, ssl = connected_pair(server_ctx, server) + data = client.wstate + counted = Ref(false) + # counting in and out + OpenSSL.countin!(client, counted) + @test counted[] && OpenSSL.writesinflight(client) == 1 + OpenSSL.endwrite!(client) + @test data[] == 0 + # once the close has set its bit, a write is refused without the word changing + Threads.atomic_or!(data, OpenSSL.CLOSING) + counted[] = false + before = data[] + OpenSSL.countin!(client, counted) + @test !counted[] && data[] == before + @test OpenSSL.closing(client) + # and counting out what was never counted in is refused, not a borrow through the + # bit; logged rather than thrown, being done in a `finally` + @test_logs (:error, r"never counted in") OpenSSL.endwrite!(client) + @test data[] == before + data[] = 0 + close(client) + close(ssl) + close(server) +end - server_ctx = OpenSSL.SSLContext(OpenSSL.TLSServerMethod(), "") - OpenSSL.ssl_use_certificate(server_ctx, cert) - OpenSSL.ssl_use_private_key(server_ctx, key) +@testset "Ticker" begin + # a one-shot timer calls back once; closed before it goes off, not at all; and + # neither logs an error (reading the timer on its end once did, on Julia 1.7) + calls = Threads.Atomic{Int}(0) + @test_logs min_level=Base.CoreLogging.Warn begin + t = OpenSSL.ticker(_ -> Threads.atomic_add!(calls, 1), 0.1) + sleep(0.5) + @test calls[] == 1 + @test !isopen(t) + t = OpenSSL.ticker(_ -> Threads.atomic_add!(calls, 1), 5.0) + close(t) + sleep(0.3) + @test calls[] == 1 + end + # an interrupt in a callback does not end the timer: it is warned about, and the + # next tick comes + calls[] = 0 + t = @test_logs (:warn, r"interrupt") match_mode=:any begin + t = OpenSSL.ticker(0.05; interval=0.05) do _ + Threads.atomic_add!(calls, 1) == 0 && throw(InterruptException()) + end + sleep(0.5) + t + end + @test calls[] >= 3 + close(t) + # a repeating timer's tick cut short is dropped, not run again at once: a watch run + # again at once would judge nothing to have moved in no time + times = Float64[] + timeslock = ReentrantLock() + started = time() + t = @test_logs (:warn, r"interrupt") match_mode=:any begin + t = OpenSSL.ticker(0.2; interval=0.2) do _ + first = Base.@lock timeslock (push!(times, time()); length(times) == 1) + first && throw(InterruptException()) + end + sleep(0.7) + t + end + close(t) + # against the timer's own schedule, not the first call, which may run late: the + # second call comes at the second tick at the earliest, not at once after the first + second = Base.@lock timeslock (length(times) >= 2 ? times[2] : 0.0) + @test second >= started + 0.35 + # a one-shot timer's callback interrupted before it got to its end runs again: the + # tick stays due until it has run to its end once + calls[] = 0 + @test_logs (:warn, r"interrupt") match_mode=:any begin + OpenSSL.ticker(0.05) do _ + Threads.atomic_add!(calls, 1) == 0 && throw(InterruptException()) + end + sleep(0.5) + end + @test calls[] == 2 + # an error in a callback is logged and ends the timer, closed + calls[] = 0 + t = @test_logs (:error, r"timer callback failed") match_mode=:any begin + t = OpenSSL.ticker(0.05; interval=0.05) do _ + Threads.atomic_add!(calls, 1) + error("from the callback") + end + sleep(0.5) + t + end + @test calls[] == 1 + @test !isopen(t) + # a repeating one calls back on each tick, and not once more after its owner closed + # it, whatever tick raced the close + calls[] = 0 + t = OpenSSL.ticker(_ -> Threads.atomic_add!(calls, 1), 0.05; interval=0.05) + sleep(0.5) + @test calls[] >= 3 + close(t) + # a callback that passed the open check just before the close, on another thread, + # may still be under way: wait until the count has stopped changing + settled = timedwait(10.0; pollint=0.1) do + c = calls[] + sleep(0.1) + calls[] == c + end + @test settled === :ok + after = calls[] + sleep(0.3) + @test calls[] == after +end +@testset "CallerNotPinned" begin + # the tasks the library starts do not pin the task that starts them to its thread, + # as `@async` does from Julia 1.7: an abort from a spawned task leaves it free + server_ctx = selfsigned_server_ctx() port, server = Sockets.listenany(ip"127.0.0.1", 20000) - handshake_done = Channel{Nothing}(1) + # an abort, a graceful close, and a handshake with a deadline, each from a spawned + # task: the last two make timers, which `Timer(cb, ...)` would pin up to Julia 1.11 + client, ssl = connected_pair(server_ctx, server) + @test fetch(Threads.@spawn (close(client, false); current_task().sticky)) == false + close(ssl) + client, ssl = connected_pair(server_ctx, server) + @test fetch(Threads.@spawn (close(client); current_task().sticky)) == false + close(ssl) + port = Sockets.getsockname(server)[2] server_task = @async begin - sock = accept(server) - ssl = OpenSSL.SSLStream(server_ctx, sock) - Sockets.accept(ssl) - put!(handshake_done, nothing) - eof(ssl) + s = OpenSSL.SSLStream(server_ctx, accept(server)) + Sockets.accept(s; timeout=30) + s + end + try + pinned = fetch(Threads.@spawn begin + c = OpenSSL.SSLStream(OpenSSL.SSLContext(OpenSSL.TLSClientMethod(), ""), + Sockets.connect(ip"127.0.0.1", port)) + Sockets.connect(c; require_ssl_verification=false, timeout=30) + close(c) + current_task().sticky + end) + @test pinned == false + finally + # should the handshake fail, leave neither the server task nor the listener + close(server) + # the server's side of the handshake may still be finishing: give it the time + # well past the server side's own deadline + @test timedwait(() -> istaskdone(server_task), 60.0) === :ok + istaskdone(server_task) && !istaskfailed(server_task) && close(fetch(server_task)) + end +end + +@testset "WatchEndsStandingCount" begin + # a write counted in that never counts out (an interrupt at the wrong moment, say) + # must not keep a graceful close waiting for good: nothing moves, and the close's + # watch gives up and ends it, whether the socket is still open or already closed + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + grace = OpenSSL.CLOSE_GRACE[] + OpenSSL.CLOSE_GRACE[] = 0.5 + try + for socket_closed_first in (false, true) + client, ssl = connected_pair(server_ctx, server) + Threads.atomic_add!(client.wstate, 1) + socket_closed_first && close(client.io) + try + closer = @async close(client) + @test timedwait(() -> istaskdone(closer), 10.0) === :ok + @test !isopen(client) + finally + # should the close hang, leave nothing behind for the testsets after + close(client, false) + close(ssl) + end + end + finally + OpenSSL.CLOSE_GRACE[] = grace + end + close(server) +end + +@testset "CloseStopsLoopingWriter" begin + # a writer calling `write` back to back must not keep a graceful close waiting: the + # writer lock does not hand itself over in order, so the close would lose to each + # next write. Writes that start after the close fail, and the close gets through. + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + client, ssl = connected_pair(server_ctx, server) + reader = @async try + while !eof(ssl) + readavailable(ssl) + end + :eof + catch ex + ex + end + # the client reads too, which takes the server's session tickets off its socket: + # closed with them unread, the kernel would reset the connection rather than end it, + # and the peer might lose the close_notify to the reset + client_reader = @async try + while !eof(client) + readavailable(client) + end + catch end + # it retries after being refused, too, on another thread when there is one: a + # refused write must not count, or a retrying writer could keep the close waiting + # until its watch gave up + writer = Threads.@spawn begin + chunk = zeros(UInt8, 16 * 1024) + refusals = 0 + while isopen(client) + try + write(client, chunk) + catch ex + ex isa Base.IOError || rethrow() + refusals += 1 + # a refusal comes back without waiting on anything: on one thread, a + # loop that never yields would keep everything else from running + yield() + end + end + refusals + end + sleep(0.5) + @test !istaskdone(writer) + closer = @async close(client) + # well before the close's watch could give up, at `CLOSE_GRACE` + @test OpenSSL.CLOSE_GRACE[] >= 5 + @test timedwait(() -> istaskdone(closer), 3.0) === :ok + @test timedwait(() -> istaskdone(writer), 10.0) === :ok + # how many refusals it met depends on the timing; that it stopped is what counts + @test istaskdone(writer) && fetch(writer) isa Int + @test OpenSSL.writesinflight(client) == 0 + awaitpeer(reader, ssl, client) do ended + # the close was graceful: the peer got the close_notify, not a cut connection + @test ended isa Base.IOError && occursin("unexpected EOF", sprint(showerror, ended)) + end + # and an empty write on a closed stream is no error, as it never was; a write that is + # not empty says the stream is closed, no longer that it is being closed + @test write(client, UInt8[]) == 0 + late = try + write(client, UInt8[1]) + nothing + catch ex + ex + end + @test late isa Base.IOError && occursin("requires ssl to be open", sprint(showerror, late)) + close(server) +end +@testset "FinishUnderDeadline" begin + # the handshake's last step (the certificate check, for `connect`) counts toward + # its deadline: one that runs past it ends in the timeout, the stream closed + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + # one handshake! call site and one kind of last step for both runs below, so that the + # first compiles what the second needs, which then has only its own rounds to fit + # in its deadline + # (@eval'd into the package for `@geterror` and the internals it names) + deadline_connect! = @eval OpenSSL (client, timeout, finish) -> + handshake!(client, :connect, timeout; finish=finish) do + @geterror client :connect ssl_connect(client.ssl) + end + # notes that the stream was open when the step began, the rounds before it having + # finished inside the deadline; then, if told to, waits for the deadline to close it + laststep(client, entered, wait) = function () + entered[] = isopen(client) + wait && timedwait(() -> !isopen(client), 30.0) + nothing + end + function attempt(timeout, wait) + # this attempt's own, not the testset's of the same names + local server_task, client, entered, err + server_task = @async begin + s = OpenSSL.SSLStream(server_ctx, accept(server)) + try + Sockets.accept(s; timeout=30) + catch + end + s + end + client = OpenSSL.SSLStream(OpenSSL.SSLContext(OpenSSL.TLSClientMethod(), ""), + Sockets.connect(ip"127.0.0.1", port)) + entered = Ref(false) + err = try + Base.invokelatest(deadline_connect!, client, timeout, + laststep(client, entered, wait)) + nothing + catch ex + ex + end + @test timedwait(() -> istaskdone(server_task), 30.0) === :ok + istaskdone(server_task) && close(fetch(server_task)) + return client, entered[], err + end + # the warm-up, well inside its deadline + client, entered, err = attempt(60.0, false) + @test entered + @test err === nothing + @test isopen(client) + close(client) + client, entered, err = attempt(3.0, true) + @test entered + @test err isa Base.IOError && occursin("timed out", sprint(showerror, err)) + @test !isopen(client) + close(server) +end + +@testset "WatchStandsDown" begin + # a watch that ticks after its graceful close handed the socket to its normal close + # stands down: it neither aborts the stream nor cuts its socket, whatever it sees; + # and one that sees nothing moved on a stream not handed off gives up + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + for handedoff in (true, false) + client, ssl = connected_pair(server_ctx, server) + watch = OpenSSL.AbortWatch(client) + Base.@lock client.lock (client.data.handedoff = handedoff) + timer = Timer(3600) + try + watch(timer) + @test client.data.aborted == !handedoff + if handedoff + @test isopen(client.io) + else + # closed outright: up to Julia 1.8 a socket reads as open until its close + # callback has run + @test timedwait(() -> !isopen(client.io), 5.0) === :ok + end + @test !isopen(timer) + finally + close(timer) + close(client, false) + close(ssl) + end + end + close(server) +end + +@testset "FailedWriteKeepsNothing" begin + # a write that ended with the socket's own error: libuv is done with it and holds + # no pointer into its chunk, which is not kept. A reset from the peer gives one, on + # Linux for sure: a peer closing with bytes unread resets the connection + if Sys.islinux() + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + client, ssl = connected_pair(server_ctx, server) + stop_writing = Threads.Atomic{Bool}(false) + parked_writer, _ = park_writer(client, stop_writing) + stop_writing[] = true + try + withkept(client) do kept + close(ssl.io) + @test timedwait(() -> istaskdone(parked_writer), 10.0) === :ok + failure = istaskdone(parked_writer) ? fetch(parked_writer) : nothing + @test failure isa Base.IOError + @test isempty(kept()) + end + finally + close(client, false) + close(ssl) + close(server) + end + end +end + +@testset "UndrainedTicket" begin + # a ticket taken and never drained (its task stopped in between) must not hold the + # tickets behind it for good: once the stream is aborted and its watch gives up, + # the socket goes, and a writer waiting for its turn behind that ticket fails + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + client, ssl = connected_pair(server_ctx, server) + data = client.data + # the orphan, taken under `ssl.lock`, as `take!` hands tickets out + Base.@lock client.lock (data.nextticket += 1) + writer = @async try + write(client, UInt8[1]) + nothing + catch ex + ex + end + queued() = Base.@lock data.cond data.waiting + @test timedwait(() -> queued() == 1, 5.0) === :ok + grace = OpenSSL.ABORT_GRACE[] + OpenSSL.ABORT_GRACE[] = 0.5 + try + close(client, false) + @test timedwait(() -> istaskdone(writer), 10.0) === :ok + @test istaskdone(writer) && fetch(writer) isa Base.IOError + @test timedwait(() -> !isopen(client.io), 10.0) === :ok + finally + OpenSSL.ABORT_GRACE[] = grace + end + close(ssl) + + # the same with the handle closed through the fallback, as without the internals + # `forceclose!` builds on: the socket's status then says closed only later, and the + # waits must end on what the cut recorded, not on that + client, ssl = connected_pair(server_ctx, server) + data = client.data + # under `ssl.lock`, as `take!` hands tickets out + Base.@lock client.lock (data.nextticket += 1) + writer = @async try + write(client, UInt8[1]) + nothing + catch ex + ex + end + @test timedwait(() -> queued() == 1, 5.0) === :ok + OpenSSL.ABORT_GRACE[] = 0.5 + OpenSSL.FORCECLOSE_FALLBACK[] = true + try + close(client, false) + @test timedwait(() -> istaskdone(writer), 10.0) === :ok + @test istaskdone(writer) && fetch(writer) isa Base.IOError + @test timedwait(() -> !isopen(client.io), 10.0) === :ok + finally + OpenSSL.FORCECLOSE_FALLBACK[] = false + OpenSSL.ABORT_GRACE[] = grace + end + close(ssl) + close(server) +end + +@testset "FailedCallAbortsAtOnce" begin + # a call that fails closes and aborts the stream in one go, under `ssl.lock`: by + # the time `@sslcall` hands back, the abort has been started, so nothing landing + # in between (another task's abort, an interrupt) can leave the stream closed with + # no abort run, or its alert behind. `@sslcall` is run by hand to look right there. + # With `data.cond` held by another task too: the claim takes no lock but `ssl.lock`, + # so the alert-less abort after it cannot claim the stream first + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + # each run its own set, so that a failure says which + @testset "contended=$contended" for contended in (false, true) + client, ssl = connected_pair(server_ctx, server) + data = client.data + # a record that does not decrypt: the peek fails, OpenSSL queues a bad_record_mac + write(ssl.io, UInt8[0x17, 0x03, 0x03, 0x00, 0x11, fill(0xaa, 17)...]) + # held by a task of its own, the lock being reentrant: the claim must not depend + # on it + held, release = Base.Event(), Base.Event() + holder = contended ? (@async Base.@lock data.cond (notify(held); wait(release))) : nothing + contended && wait(held) + # in a task of its own, under a deadline: a claim that took `data.cond` would stay + # parked behind the holder, which is a failure to report, not a hang + run = @async try + # a `let`: `@eval` runs at the module's top level, and would leave a global + # behind + @eval OpenSSL let client = $client, contended = $contended + local ret, pending, err + # a message another call left for this task: not the failed call's to + # take, nor to show + task_local_storage(:openssl_err, "stale") + for round in 1:100 + eof(client.io) && error("the socket ended before the record failed") + ret, pending, err = @sslcall client :peek ccall((:SSL_peek_ex, libssl), Cint, + (SSL, Ptr{UInt8}, Csize_t, Ptr{Csize_t}), client.ssl, client.peekbuf, 1, client.peekbytes) + err === nothing || break + # its drain would wait for `data.cond`, held until this is done + contended && pending !== nothing && + error("a round before the failing one produced output") + finish_sslcall!(client, ret, pending, err) + round == 100 && error("the record did not fail") + end + aborted = getfield(client, :data).aborted + # another abort now does nothing more + close(client, false) + res = try + finish_sslcall!(client, ret, pending, err) + :no_error + catch ex + ex + end + (aborted, res, get(task_local_storage(), :openssl_err, nothing)) + end + catch ex + ex + end + out = boundedfetch(run, :stalled) + if contended + notify(release) + wait(holder) + end + @test out !== :stalled + if out === :stalled + # the run, released, is let finish should it have waited behind the holder; + # else its socket is cut, which wakes it wherever it waits (it can hold + # `client.lock`, which an abort would wait for, so it is not aborted) + contended && timedwait(() -> istaskdone(run), 10.0) + istaskdone(run) || OpenSSL.cut!(data) + end + @test out isa Tuple + aborted, res, left = out isa Tuple ? out : (false, :not_run, nothing) + @test aborted + @test res isa Base.IOError + # the reason reaches the message, and leaves the thread's error queue: nothing + # for a later call to take for its own + @test res isa Base.IOError && occursin("bad record mac", lowercase(res.msg)) + @test ccall((:ERR_peek_error, OpenSSL.libcrypto), Culong, ()) == 0 + @test res isa Base.IOError && !occursin("stale", res.msg) + @test left == "stale" + @test timedwait(() -> Base.@lock(data.cond, OpenSSL.drained(data)), 10.0) === :ok + # and the peer got the alert, not a bare end of the connection; only asked once + # the abort ran, the peer having nothing to read otherwise + if aborted + alerted = boundedread(ssl, client) do + try + eof(ssl) + false + catch ex + ex isa Base.IOError && !occursin("unexpected EOF", sprint(showerror, ex)) + end + end + @test alerted === true + end + close(ssl) + end + close(server) +end + +@testset "AcceptTimeout" begin + # a client that connects and then says nothing + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + server_task = @async begin + ssl = OpenSSL.SSLStream(server_ctx, accept(server)) + try + Sockets.accept(ssl; timeout=0.5) + nothing + catch ex + # the timeout closes the stream + (ex, isopen(ssl)) + finally + close(ssl) + end + end + silent = Sockets.connect(ip"127.0.0.1", port) + # fetched only once done, a stalled handshake failing the test rather than hanging + # it; and a handshake that completed (`nothing`) failing it too, not erroring + r = boundedfetch(server_task, :stalled; timeout=10.0) + @test r !== :stalled + err, open = r isa Tuple ? r : (r, true) + @test err isa Base.IOError + @test occursin("timed out", sprint(showerror, err)) + @test !open + close(silent) + + # a client that goes away mid-handshake: the same EOFError as without a timeout, + # not wrapped in the error of the task that waited on the socket + server_task = @async begin + ssl = OpenSSL.SSLStream(server_ctx, accept(server)) + try + Sockets.accept(ssl; timeout=10) + nothing + catch ex + # the peer going away closes the stream too + (ex, isopen(ssl)) + finally + close(ssl) + end + end + leaver = Sockets.connect(ip"127.0.0.1", port) + sleep(0.2) + close(leaver) + r = boundedfetch(server_task, :stalled; timeout=10.0) + @test r !== :stalled + err, open = r isa Tuple ? r : (r, true) + @test err isa EOFError + @test !open + # a timeout that is not a positive number is refused before anything happens + probe = OpenSSL.SSLStream(server_ctx, Sockets.connect(ip"127.0.0.1", port)) + @test_throws ArgumentError Sockets.accept(probe; timeout=NaN) + @test_throws ArgumentError Sockets.accept(probe; timeout=0) + @test_throws ArgumentError Sockets.accept(probe; timeout=-1) + @test isopen(probe) + close(probe) + # the probe's connection is still in the listen backlog: take it out of the way + close(accept(server)) + + # the client side has the same deadline: a server that accepts and then says nothing + mute = @async accept(server) client = OpenSSL.SSLStream(OpenSSL.SSLContext(OpenSSL.TLSClientMethod(), ""), Sockets.connect(ip"127.0.0.1", port)) - Sockets.connect(client; require_ssl_verification=false) - take!(handshake_done) - # take whatever the server sent after the handshake (session tickets) off the - # socket, so that closing it sends a FIN rather than a RST - sleep(0.2) - readavailable(client.io) - # a record header announcing 100 bytes, followed by only 3 of them, then close - write(client.io, UInt8[0x17, 0x03, 0x03, 0x00, 0x64, 0xaa, 0xbb, 0xcc]) + err = try + Sockets.connect(client; require_ssl_verification=false, timeout=0.5) + nothing + catch ex + ex + end + @test err isa Base.IOError + @test occursin("timed out", sprint(showerror, err)) + @test !isopen(client) + close(fetch(mute)) + close(server) +end + +@testset "InterruptedHandshakeCleanup" begin + # an exception thrown into a task whose failed handshake is waiting to abort the + # stream, an interrupt or any other: it goes on at once, and the abort is done all the + # same, on a task of the library's, with nothing logged as a failure + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + for injected in (InterruptException(), ErrorException("cancelled")) + peer_task = @async accept(server) + client = OpenSSL.SSLStream(OpenSSL.SSLContext(OpenSSL.TLSClientMethod(), ""), + Sockets.connect(ip"127.0.0.1", port)) + peer = fetch(peer_task) + # the task's logger, and its tasks', collects what the library logs + logger = Test.TestLogger(; min_level=Base.CoreLogging.Warn) + handshaker = Base.CoreLogging.with_logger(logger) do + @async try + Sockets.connect(client; require_ssl_verification=false) + nothing + catch ex + ex + end + end + # the ClientHello is out: the handshake waits for the reply + @test !eof(peer) + lock(client.lock) + try + # the peer goes, the handshake fails, and its abort waits for the lock + close(peer) + sleep(1.0) + @test !istaskdone(handshaker) + schedule(handshaker, injected; error=true) + sleep(0.5) + finally + unlock(client.lock) + end + @test timedwait(() -> istaskdone(handshaker), 10.0) === :ok + @test istaskdone(handshaker) && fetch(handshaker) === injected + # on the task the abort was left to, should it have been + @test timedwait(() -> !isopen(client), 10.0) === :ok + @test timedwait(() -> !isopen(client.io), 10.0) === :ok + @test isempty(logger.logs) + end + close(server) +end + +@testset "SurelyLocked" begin + # a cleanup section's lock, taken surely: an exception thrown into the task while it + # waits for it goes on at once, and the section, and what is to follow it, are left + # to a task of the library's, which runs them once the lock is free + for l in (ReentrantLock(), Threads.Condition()), then in (false, true) + ran = Threads.Atomic{Int}(0) + injected = ErrorException("cancelled") + lock(l) + t = @async try + if then + OpenSSL.surely(() -> (ran[] = 1), l; then=r -> (ran[] = r + 1)) + else + OpenSSL.surely(() -> (ran[] = 1), l) + end + nothing + catch ex + ex + end + try + sleep(0.5) + @test !istaskdone(t) + schedule(t, injected; error=true) + @test timedwait(() -> istaskdone(t), 10.0) === :ok + @test istaskdone(t) && fetch(t) === injected + # not while the lock is held + sleep(0.2) + @test ran[] == 0 + finally + unlock(l) + end + @test timedwait(() -> ran[] == (then ? 2 : 1), 10.0) === :ok + @test timedwait(() -> !islocked(l), 10.0) === :ok + # nothing thrown in: on the caller's task, `then` on the section's value + @test OpenSSL.surely(() -> 42, l) == 42 + @test OpenSSL.surely(() -> 42, l; then=r -> r + 1) == 43 + end + # what is left to a task of the library's and fails is logged, not lost + logger = Test.TestLogger(; min_level=Base.CoreLogging.Error) + t = Base.CoreLogging.with_logger(() -> OpenSSL.leave(() -> error("left and failed")), logger) + @test timedwait(() -> istaskdone(t), 10.0) === :ok + @test any(r -> occursin("left to a task of the library failed", r.message), logger.logs) + # as is an interrupt that what was left throws itself, once, rather than taken for one + # that landed before the hold and run again + runs = Threads.Atomic{Int}(0) + logger = Test.TestLogger(; min_level=Base.CoreLogging.Error) + t = Base.CoreLogging.with_logger(logger) do + OpenSSL.leave(() -> (Threads.atomic_add!(runs, 1); throw(InterruptException()))) + end + @test timedwait(() -> istaskdone(t), 10.0) === :ok + @test runs[] == 1 + @test any(r -> occursin("left to a task of the library was interrupted", r.message), logger.logs) +end + +@testset "InterruptedTake" begin + # taking a call's records waits for nothing: a writer takes its ticket while another + # task holds `data.cond`, so nothing thrown into it can land between making records + # and ticketing them. Stopped then, waiting for `data.cond` in `drain!`, its records + # are lost, and the stream with them; none of them reaches the peer + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + client, ssl = connected_pair(server_ctx, server) + data = client.data + injected = ErrorException("cancelled") + before = data.nextticket + lock(data.cond) + writer = @async try + write(client, b"first") + nothing + catch ex + ex + end + try + # ticketed with `data.cond` held here: `take!` did not wait for it + @test timedwait(() -> data.nextticket == before + 1, 10.0) === :ok + sleep(0.2) + @test !istaskdone(writer) + schedule(writer, injected; error=true) + sleep(0.2) + finally + unlock(data.cond) + end + @test timedwait(() -> istaskdone(writer), 10.0) === :ok + @test istaskdone(writer) && fetch(writer) === injected + @test timedwait(() -> !isopen(client), 10.0) === :ok + @test_throws Base.IOError write(client, b"second") + reader = @async try + got = UInt8[] + while !eof(ssl) + append!(got, readavailable(ssl)) + end + String(got) + catch ex + ex + end + awaitpeer(reader, ssl, client) do r + # nothing, or the socket going (a reset, the client not having read what the + # server sent after the handshake); not a record, nor an SSL error about one + @test r == "" || (r isa Base.IOError && !occursin("SSL", r.msg)) + end + close(server) +end + +# a logger that throws on every message, as one that does not catch its own errors +# passes them on; noting the messages it was given +struct ThrowingLogger <: Base.CoreLogging.AbstractLogger + seen::Vector{String} + lock::ReentrantLock +end +ThrowingLogger() = ThrowingLogger(String[], ReentrantLock()) +Base.CoreLogging.min_enabled_level(::ThrowingLogger) = Base.CoreLogging.Debug +Base.CoreLogging.shouldlog(::ThrowingLogger, args...) = true +Base.CoreLogging.catch_exceptions(::ThrowingLogger) = false +function Base.CoreLogging.handle_message(logger::ThrowingLogger, level, message, args...; kwargs...) + Base.@lock logger.lock push!(logger.seen, string(message)) + error("the logger threw") +end +saw(logger::ThrowingLogger, what) = Base.@lock logger.lock any(m -> occursin(what, m), logger.seen) + +@testset "ThrowingLogger" begin + # what the library logs on its cleanup paths cannot stop them: a cancelled in-flight + # write through the fallback close, which logs before it schedules the close, with + # the writer (and the tasks it starts) logging to a logger that throws + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + client, ssl = connected_pair(server_ctx, server) + stop_writing = Threads.Atomic{Bool}(false) + logger = ThrowingLogger() + parked_writer, _ = Base.CoreLogging.with_logger(logger) do + park_writer(client, stop_writing) + end + stop_writing[] = true + OpenSSL.FORCECLOSE_FALLBACK[] = true + try withkept(client) do kept + schedule(parked_writer, InterruptException(); error=true) + @test timedwait(() -> istaskdone(parked_writer), 5.0) === :ok + @test istaskdone(parked_writer) && fetch(parked_writer) isa InterruptException + # the fallback's log reached, and the fallback on past it: the chunk kept until + # the close is through + @test saw(logger, "could not close the socket handle outright") + @test timedwait(() -> length(kept()) == 1, 5.0) === :ok + data = client.data + @test Base.@lock(data.cond, data.cut) + # the peer reads, the queued write goes out, the close the fallback scheduled + # gets through + reader = @async try + while !eof(ssl.io) + readavailable(ssl.io) + end + catch + end + @test timedwait(() -> !isopen(client.io), 30.0) === :ok + @test timedwait(() -> istaskdone(reader), 30.0) === :ok + end + finally + OpenSSL.FORCECLOSE_FALLBACK[] = false + end + close(ssl) + + # a graceful close whose close_notify cannot go out, the socket gone under it: the + # close logs that, on the caller's task, and returns all the same + client, ssl = connected_pair(server_ctx, server) close(client.io) + logger = ThrowingLogger() + Base.CoreLogging.with_logger(logger) do + @test (close(client); true) + end + # the log was reached, and threw + @test saw(logger, "close_notify not sent") + @test !isopen(client) + close(ssl) + close(server) +end - # the spin never yields, so on one thread a hang here would starve this task too; - # a task that does not finish is reported by the timeout, a spinning one by CI - @test timedwait(() -> istaskdone(server_task), 30.0) === :ok - @test istaskdone(server_task) && fetch(server_task) === true +@testset "LostRecordNoCloseNotify" begin + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + # a graceful close on a stream that lost a record produces no close_notify: it would + # be refused, and would mark the session as shut down cleanly + client, ssl = connected_pair(server_ctx, server) + data = client.data + Base.@lock data.cond (data.lostfrom = data.nextticket) + @test Base.@lock(client.lock, OpenSSL.closelocked!(client)) === nothing + close(client, false) + close(ssl) + # and through the public close: the stream closed, and the peer gets no close_notify + # (which the ticket order would refuse anyway: that none is produced, keeping the + # session from being marked as shut down cleanly, is the case above's to pin) + client, ssl = connected_pair(server_ctx, server) + data = client.data + Base.@lock data.cond (data.lostfrom = data.nextticket) close(client) + @test !isopen(client) + # nothing clean-looking reaches the peer: a bare end, or a reset (the client not + # having read what the server sent after the handshake), not the "unexpected EOF" a + # close_notify would show as + # read from a task of its own, under a deadline: a socket left open would otherwise + # hang the suite rather than fail this + reader = @async try + eof(ssl) + catch ex + ex + end + awaitpeer(reader, ssl, client) do r + @test r === true || (r isa Base.IOError && !occursin("unexpected EOF", r.msg)) + end close(server) end From 3b126e12a6d19883dc22d5db3bddc9d20095cff5 Mon Sep 17 00:00:00 2001 From: Krystian Gulinski Date: Wed, 30 Sep 2026 09:45:27 +0200 Subject: [PATCH 15/25] test: CloseNotifyEOF and the fallback case work on Windows and macOS - CloseNotifyEOF: the client reads before closing, taking the server's session tickets off its socket; closed with them unread, Windows resets the connection and the server saw ECONNRESET instead of the close_notify. - CancelledInFlightWriter's fallback case ends its raw peer reader by closing the server's socket, once the chunk being let go shows the client's close got through: the macOS runners deliver neither a FIN nor a reset to that reader. Co-Authored-By: Claude Opus 5.5 --- test/runtests.jl | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/runtests.jl b/test/runtests.jl index f2df64d..4fc850c 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -1029,6 +1029,17 @@ end server_ctx = selfsigned_server_ctx() port, server = Sockets.listenany(ip"127.0.0.1", 20000) client, ssl = connected_pair(server_ctx, server) + # the client reads first, which takes the server's session tickets off its socket: + # closed with them unread, the kernel may reset the connection rather than end it + # (Windows does), and the peer lose the close_notify to the reset + client_reader = @async try + while !eof(client) + readavailable(client) + end + catch + end + sleep(0.2) + @test timedwait(() -> bytesavailable(client.io) == 0, 10.0) === :ok close(client) err = boundedread(ssl, client) do try @@ -1243,6 +1254,10 @@ end if length(k) == 1 @test timedwait(() -> !held(only(k)), 30.0) === :ok end + # the client's close got through, which the chunk let go says; the reader is + # ended from this side, the peer's end not reaching it on every platform (macOS + # may deliver neither a FIN nor a reset here) + close(ssl.io) @test timedwait(() -> istaskdone(reader), 30.0) === :ok end finally From 8545e4e30c18ae9bfd778638bd188a52107d3e0f Mon Sep 17 00:00:00 2001 From: Krystian Gulinski Date: Wed, 30 Sep 2026 10:02:14 +0200 Subject: [PATCH 16/25] test: ThrowingLogger's fallback case ends its raw peer reader from the server side - As in CancelledInFlightWriter: once the client's socket is closed, the server closes its own to end the raw reader, the macOS runners delivering neither a FIN nor a reset to it. Co-Authored-By: Claude Opus 5.5 --- test/runtests.jl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/runtests.jl b/test/runtests.jl index 4fc850c..bf5ef47 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -2082,6 +2082,9 @@ saw(logger::ThrowingLogger, what) = Base.@lock logger.lock any(m -> occursin(wha catch end @test timedwait(() -> !isopen(client.io), 30.0) === :ok + # the reader ended from this side, the peer's end not reaching it on every + # platform (macOS may deliver neither a FIN nor a reset here) + close(ssl.io) @test timedwait(() -> istaskdone(reader), 30.0) === :ok end finally From a2c6a1e8cb2657f3bb453fb487017580fe04a9e2 Mon Sep 17 00:00:00 2001 From: krynju Date: Wed, 30 Sep 2026 10:38:15 +0200 Subject: [PATCH 17/25] chore: bump version to 1.7.0 The write BIO only buffers now, so callers driving SSL_* on ssl.ssl directly (e.g. their own SSL_accept loop) no longer get their output sent; a minor bump keeps "1.6" compat from picking this up. Co-Authored-By: Claude Opus 5.5 --- Project.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Project.toml b/Project.toml index a4706cd..d7f6b8b 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "OpenSSL" uuid = "4d8831e6-92b7-49fb-bdf8-b643e874388c" -version = "1.6.2" +version = "1.7.0" authors = ["Greg Lapinski ", "Jacob Quinn "] [deps] From 4fb5008f1416aacd4081e8aeb2a830246b10e62a Mon Sep 17 00:00:00 2001 From: krynju Date: Wed, 30 Sep 2026 11:42:41 +0200 Subject: [PATCH 18/25] perf: write one record per SSL_write_ex, and reuse the sent chunk as the next buffer The write BIO grew a fresh buffer for the whole output of each SSL_write_ex, up to 1 MiB of plaintext per call, by repeated resize!: about 4 bytes allocated per byte written, and 1 MiB writes at under half of main's throughput. Cap each call at one record (16 KiB), and keep a chunk that reached the socket as the next buffer, so a long write reuses one buffer. Loopback, one thread, main -> before -> after: 1 MiB writes 800-850 -> 344-386 -> 735-758 MB/s alloc/1 MiB write (reader included) 1032 -> 4093 -> 1042 kB 64 B alloc/write 27 -> 219 -> 75 B Co-Authored-By: Claude Opus 5.5 --- src/ssl.jl | 55 +++++++++++++++++++++++++++++++++++++++++------- test/runtests.jl | 22 +++++++++++++++++++ 2 files changed, 69 insertions(+), 8 deletions(-) diff --git a/src/ssl.jl b/src/ssl.jl index f14b9d1..e053e8f 100644 --- a/src/ssl.jl +++ b/src/ssl.jl @@ -112,9 +112,13 @@ mutable struct BIOStreamData # the socket handle was closed outright (see `cut!`): no ticket is written any more, # and whatever waits for one stops waiting. Under `cond` cut::Bool + # a chunk that reached the socket, emptied, for `take!` to hand the write BIO as its + # next buffer instead of growing a new one record by record. Only a sent chunk: libuv + # is done with it then, and nothing else holds it. Under `cond` + spare::Union{Nothing, Vector{UInt8}} end -BIOStreamData(io::TCPSocket) = BIOStreamData(io, UInt8[], Threads.Condition(), 0, 0, Set{Int}(), 0, typemax(Int), false, false, false) +BIOStreamData(io::TCPSocket) = BIOStreamData(io, UInt8[], Threads.Condition(), 0, 0, Set{Int}(), 0, typemax(Int), false, false, false, nothing) """ take!(data::BIOStreamData) -> Union{PendingWrite, Nothing} @@ -131,7 +135,7 @@ function Base.take!(data::BIOStreamData) # between: no lock either, `ssl.lock` serialising the tickets (see `nextticket`). # An interrupt at the allocation leaves the records where they are, for `@sslcall` # to give up together with the stream - fresh = UInt8[] + fresh = takespare!(data) ticket = data.nextticket data.nextticket = ticket + 1 chunk = data.buf @@ -139,6 +143,32 @@ function Base.take!(data::BIOStreamData) return PendingWrite(chunk, ticket) end +# the spare chunk (see `spare`), or a new buffer when there is none or `cond` is held: +# `take!` runs under `ssl.lock` and must not wait for a writer +function takespare!(data::BIOStreamData) + spare = nothing + if trylock(data.cond) + try + spare = data.spare + data.spare = nothing + finally + unlock(data.cond) + end + end + return spare === nothing ? UInt8[] : spare +end + +# keeps a chunk that reached the socket as the spare (see `spare`), unless there is one +# already or it is larger than one record of a write (a handshake flight, say), which +# would hold that memory for as long as the stream lives; under `cond` +function keepspare!(data::BIOStreamData, chunk::Vector{UInt8}) + if data.spare === nothing && length(chunk) <= SPARE_MAX + empty!(chunk) + data.spare = chunk + end + return nothing +end + """ Writes a chunk `take!` returned to the socket. Must be called without `ssl.lock` held. Chunks go out in ticket order, so a task whose records came after another @@ -195,7 +225,7 @@ function drain!(data::BIOStreamData, pending::PendingWrite) # bound anew for the closures: captured as they are, having been assigned in the # `try`, they would be boxed on every write let sent = sent, ticket = pending.ticket, keep = finished ? nothing : pending.chunk, - failure = failure + failure = failure, chunk = pending.chunk # The write failed, or the task was cancelled inside it. The record is lost # either way, and with it the stream. A cancelled write is moreover still # queued in libuv, with a pointer into the chunk that nothing keeps alive any @@ -217,7 +247,7 @@ function drain!(data::BIOStreamData, pending::PendingWrite) try sent || cut!(data, keep, failure) finally - passon!(data, ticket, sent) + passon!(data, ticket, sent, chunk) end end end @@ -259,9 +289,12 @@ function cut!(data::BIOStreamData, keep=nothing, cause=nothing) end # passes the turn on after a drain; a chunk not sent is given up as `abandon!` does -function passon!(data::BIOStreamData, ticket::Int, sent::Bool) +function passon!(data::BIOStreamData, ticket::Int, sent::Bool, chunk::Vector{UInt8}) sent || return abandon!(data, ticket) - surely(() -> passturn!(data), data.cond) + surely(data.cond) do + passturn!(data) + keepspare!(data, chunk) + end return nothing end @@ -1174,8 +1207,14 @@ function socketeof(ssl::SSLStream) end # the write BIO buffers the whole output of one `SSL_write_ex` before `drain!` moves it -# to the socket, so cap how much plaintext goes into a single call to bound that buffer -const SSL_WRITE_CHUNK = UInt(1 << 20) +# to the socket, so cap how much plaintext goes into a single call to bound that buffer. +# One TLS record's worth: the chunk is then one record, which fits the spare buffer (see +# `keepspare!`), so a long write reuses one buffer instead of growing a new one to the +# size of the call +const SSL_WRITE_CHUNK = UInt(16 * 1024) +# the largest chunk kept as the spare: a whole `SSL_WRITE_CHUNK` record, with room for +# its header, padding and tag, and for a KeyUpdate or an alert riding along +const SPARE_MAX = Int(SSL_WRITE_CHUNK) + 1024 function Base.unsafe_write(ssl::SSLStream, in_buffer::Ptr{UInt8}, in_length::UInt) # nothing to write: nothing to refuse either, whatever state the stream is in diff --git a/test/runtests.jl b/test/runtests.jl index bf5ef47..6abf7ec 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -1317,6 +1317,28 @@ end close(server) end +@testset "SpareBuffer" begin + # a chunk that reached the socket becomes the write BIO's next buffer; the peer + # still reads every write whole and in order, across sizes around a record's and a + # write of many records + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + client, ssl = connected_pair(server_ctx, server) + payloads = [rand(UInt8, n) for n in (1, 100, 16383, 16384, 16385, 50_000, 2^20 + 7, 3)] + expected = reduce(vcat, payloads) + reader = @async read!(ssl, Vector{UInt8}(undef, length(expected))) + for payload in payloads + write(client, payload) + end + @test awaitread(reader, ssl, client) == expected + # the last write's chunk is kept, emptied, for the next call + spare = client.data.spare + @test spare isa Vector{UInt8} && isempty(spare) + close(client) + close(ssl) + close(server) +end + @testset "Ticker" begin # a one-shot timer calls back once; closed before it goes off, not at all; and # neither logs an error (reading the timer on its end once did, on Julia 1.7) From 543711cbcf79659c2eeab4e7328aa4c9dc30d896 Mon Sep 17 00:00:00 2001 From: krynju Date: Wed, 30 Sep 2026 11:56:03 +0200 Subject: [PATCH 19/25] test: park_writer writes more than the kernel buffers grow to With one record per SSL_write_ex, a parked write of 1.5 chunks (24 KiB) has only one record queued in libuv. Windows and macOS keep growing the socket buffers for a peer that does not read, so that write completed of itself and CancelledCloser and AbortDuringGracefulClose saw it finish. Write 32 MiB per call instead. Co-Authored-By: Claude Opus 5.5 --- test/runtests.jl | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index 6abf7ec..de2b75a 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -765,11 +765,14 @@ function awaitpeer(check, reader, ssl, client) end # writes until the peer's receive window is full and the writer parks on the socket; -# returns the task and the count of completed writes. Each write is more than what goes -# to OpenSSL in one call, so that the parked one has a chunk still to go +# returns the task and the count of completed writes. Each write is many times what goes +# to OpenSSL in one call, so that the parked one has chunks still to go, and more than +# the kernel's send and receive buffers grow to (Windows and macOS keep growing them for +# a peer that does not read): a parked write that the buffers could still take would +# complete of itself, and the tests need it to stay parked until they end it function park_writer(client, stop_writing) written = Threads.Atomic{Int}(0) - chunk = zeros(UInt8, OpenSSL.SSL_WRITE_CHUNK + OpenSSL.SSL_WRITE_CHUNK รท 2) + chunk = zeros(UInt8, max(32 * 2^20, 3 * OpenSSL.SSL_WRITE_CHUNK)) writer = @async try while !stop_writing[] write(client, chunk) From 04bffd45cb365276576e09e29e41ec2446ed9146 Mon Sep 17 00:00:00 2001 From: krynju Date: Wed, 30 Sep 2026 12:09:27 +0200 Subject: [PATCH 20/25] test: CloseWithQueuedWriter reads on the client, so its close does not reset The client never read the server's session tickets, so closing its socket with them unread made Linux reset the connection, and the server lost the tail of the writes it had not read yet along with the close_notify. With 32 MiB parked writes the server lags far enough behind for that to happen (seen on LTS with OpenSSL 1.1). The client reads, as in CloseNotifyEOF. Co-Authored-By: Claude Opus 5.5 --- test/runtests.jl | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/runtests.jl b/test/runtests.jl index de2b75a..3764682 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -1070,6 +1070,15 @@ end server_ctx = selfsigned_server_ctx() port, server = Sockets.listenany(ip"127.0.0.1", 20000) client, ssl = connected_pair(server_ctx, server) + # the client reads too, which takes the server's session tickets off its socket: + # closed with them unread, the kernel resets the connection rather than ending it, + # and the peer loses what it had not read yet of the writes, and the close_notify + client_reader = @async try + while !eof(client) + readavailable(client) + end + catch + end stop_writing = Threads.Atomic{Bool}(false) parked_writer, written = park_writer(client, stop_writing) @@ -1114,6 +1123,8 @@ end @test occursin("unexpected EOF", sprint(showerror, err)) end close(ssl) + # ended by the client's own close + @test timedwait(() -> istaskdone(client_reader), 30.0) === :ok close(server) end From fb331be7e61207c04463b78ba2d665a8eb555321 Mon Sep 17 00:00:00 2001 From: krynju Date: Fri, 2 Oct 2026 16:22:55 +0200 Subject: [PATCH 21/25] Keep the 1.x contracts: one-round accept, raw SSL calls; clean up when a reply task cannot be made Review round two. A minor bump protects nobody, "1.6" compat admits 1.7.0, so the contracts stay as they were instead: - Sockets.accept(ssl) without timeout is one non-blocking round again, throwing OpenSSLError when it needs more bytes, so the retry loops written to it, with their own deadline between calls, work as on main. The whole handshake is accept(ssl; timeout=Inf) or with a deadline. - A call made on ssl.ssl from outside the package (a caller's own SSL_accept loop, ssl_accept) has its output written to the socket by the write BIO callback itself, as before: the callback tells the package's own calls by BIOStreamData.incall, set around each under ssl.lock, and outside one it waits for the ticketed records to be through, so the records keep their order, then writes. ssl_accept(::SSL) is restored unchanged. - finish_sslcall!: when the task that sends a read's reply cannot be made or scheduled, the reply's ticket is given up and the stream aborted, so the writers after it fail instead of waiting for its turn for good. Tests: LegacyAccept, RawSSLCalls (TLSStreams 0.2.0's raw loop also completes the handshake against this), DetachedDrainNotScheduled, via an ONBACKGROUND test hook rather than a method overwrite. Co-Authored-By: Claude Fable 5.1 --- src/ssl.jl | 235 ++++++++++++++++++++++++++++++++++++++--------- test/runtests.jl | 199 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 388 insertions(+), 46 deletions(-) diff --git a/src/ssl.jl b/src/ssl.jl index e053e8f..1103368 100644 --- a/src/ssl.jl +++ b/src/ssl.jl @@ -69,6 +69,14 @@ not the caller's: a read's reply goes out from a task of its own (see `abortlocked!`), and the close_notify of a dropped stream whose socket outlived it goes out from the finalizer's task (see `finalize!`). +A call made on `ssl.ssl` from outside the package (a caller's own `SSL_accept` loop, +`ssl_accept`) is not one of those: the callback tells by `incall`, and writes what such +a call produces to the socket itself, behind the records the package's own calls have +tickets for, under whatever locks the caller holds, as every write was before 1.6.2 +(see `rawwrite`): one parked on a peer that does not read blocks there, unbounded by +the package, as every write did then. Such a call has to hold `ssl.lock`, as it always +had to. + Every SSL call runs under `ssl.lock` and takes its own output before releasing it, so `buf` is empty whenever a call starts (a close cut short between producing its close_notify and taking it leaves it there, for the abort that follows to empty) and @@ -116,9 +124,15 @@ mutable struct BIOStreamData # next buffer instead of growing a new one record by record. Only a sent chunk: libuv # is done with it then, and nothing else holds it. Under `cond` spare::Union{Nothing, Vector{UInt8}} + # an SSL call of the package's own is in progress, which takes what the write BIO is + # given with `take!`: set around each, under `ssl.lock` (see `@sslcall` and + # `closerest!`). Outside one, the callback serves a call made on `ssl.ssl` from + # outside the package, which nothing of the package's would send for, and writes to + # the socket itself, as it did before 1.6.2 (see `on_bio_stream_write`) + incall::Bool end -BIOStreamData(io::TCPSocket) = BIOStreamData(io, UInt8[], Threads.Condition(), 0, 0, Set{Int}(), 0, typemax(Int), false, false, false, nothing) +BIOStreamData(io::TCPSocket) = BIOStreamData(io, UInt8[], Threads.Condition(), 0, 0, Set{Int}(), 0, typemax(Int), false, false, false, nothing, false) """ take!(data::BIOStreamData) -> Union{PendingWrite, Nothing} @@ -313,6 +327,8 @@ end function abandon!(data::BIOStreamData, ticket::Int) surely(data.cond) do lose!(data, ticket) + # given up once already, and passed over since: nothing left to consume + ticket < data.turn && return if data.turn == ticket passturn!(data) else @@ -337,6 +353,47 @@ end # marked closed, `nextticket` being otherwise `take!`'s, under `ssl.lock` drained(data::BIOStreamData) = data.turn == data.nextticket +# writes a record made by a call from outside the package (see `incall`) to the socket, +# from the write BIO callback, under whatever locks the caller holds, as before 1.6.2. +# Once every record the package's own calls ticketed is through, so that the records +# keep the order OpenSSL made them in. Blocks while the peer's window is full, and +# nothing of the package's can bound that, every close and watch needing `ssl.lock` +# first: the caller bounds it as it did then, by closing the socket. Returns what the +# BIO callback does: the count, or 0 for a failure, which fails the SSL call. A failure +# leaves the stream unusable (OpenSSL takes a BIO failure as fatal), and a write that +# did not complete may be queued in libuv still, with a pointer into OpenSSL's own +# buffer, which `SSL_free` would free under it: so the socket is cut, which cancels the +# write, and the loss recorded, and nothing more goes out on this stream. An interrupt, +# which cannot be thrown through OpenSSL's frames, is said to be lost +function rawwrite(data::BIOStreamData, in::Ptr{Cchar}, inlen::Cint)::Cint + written = 0 + sent = false + try + if awaitdrained(data) + written = unsafe_write(data.io, in, inlen) + sent = true + end + catch ex + ex isa InterruptException && lostinterrupt() + finally + sent || holdinterrupts(() -> cut!(data)) + end + return sent ? Cint(written) : Cint(0) +end + +# waits until every ticket handed out is through, for a write that takes no ticket (see +# `incall`); false once nothing more may be sent on the socket, it having been cut or a +# record lost. Takes `cond`; the caller holds `ssl.lock`, which keeps `nextticket` still +function awaitdrained(data::BIOStreamData) + Base.@lock data.cond begin + while true + (data.cut || data.lostfrom != typemax(Int)) && return false + drained(data) && return true + wait(data.cond) + end + end +end + function on_bio_stream_read(bio::BIO, out::Ptr{Cchar}, outlen::Cint) try bio_clear_flags(bio) @@ -359,13 +416,18 @@ function on_bio_stream_write(bio::BIO, in::Ptr{Cchar}, inlen::Cint)::Cint try data = bio_get_data(bio) if data isa BIOStreamData - # buffer only; the caller of the SSL call this runs inside takes it with - # `take!` and writes it to the socket with `drain!` once `ssl.lock` is free - buf = data.buf - n = length(buf) - resize!(buf, n + inlen) - GC.@preserve buf unsafe_copyto!(pointer(buf, n + 1), Ptr{UInt8}(in), Int(inlen)) - return inlen + if data.incall + # buffer only; the caller of the SSL call this runs inside takes it with + # `take!` and writes it to the socket with `drain!` once `ssl.lock` is free + buf = data.buf + n = length(buf) + resize!(buf, n + inlen) + GC.@preserve buf unsafe_copyto!(pointer(buf, n + 1), Ptr{UInt8}(in), Int(inlen)) + return inlen + end + # a call made on `ssl.ssl` from outside the package (see `incall`): written to + # the socket here, as before 1.6.2 + return rawwrite(data, in, inlen) end written = unsafe_write(data::IO, in, inlen) return Cint(written) @@ -668,9 +730,33 @@ function ssl_connect(ssl::SSL) ssl) end -# gone in 1.6.2: with the write BIO buffering, its handshake output never reached the -# socket, and working on the bare handle it had no way to send it -ssl_accept(::SSL) = error("ssl_accept was removed: use Sockets.accept(::SSLStream)") +""" + ssl_accept(ssl::SSL) + +One round of the server side of the handshake on the bare SSL object, as it has always +been: `SSL_accept` once, `OpenSSLError` thrown when that did not complete the handshake +(most often it needs more bytes from the peer first), read ahead set once it did. What +the round produces reaches the socket from the write BIO callback itself, as a call +made from outside the package's own (see `BIOStreamData`); the caller holds `ssl.lock`. +Internal: `Sockets.accept(::SSLStream)` is the way to run the handshake. +""" +function ssl_accept(ssl::SSL) + if (ret = ccall( + (:SSL_accept, libssl), + Cint, + (SSL,), + ssl)) != 1 + throw(OpenSSLError(ret)) + end + + readahead!(ssl) + return nothing +end + +# read ahead: a recommended optimization when an SSL connection is only ever read from +# sequentially, which it is, there being no internal buffering of decrypted bytes. Set +# once the handshake is done +readahead!(ssl::SSL) = ccall((:SSL_set_read_ahead, libssl), Cvoid, (SSL, Cint), ssl, Cint(1)) # queues the close_notify. Internal, as `ssl_connect`: `close(::SSLStream)` is the way # to shut a stream down @@ -791,7 +877,14 @@ else return task end end -background(f) = schedule(unscheduled(f)) +# the tests' hook (see `ONKEEP`): called with `f` before the task is made, and what it +# throws stands for a failure to make or schedule the task +const ONBACKGROUND = Ref{Any}(nothing) +function background(f) + hook = ONBACKGROUND[] + hook === nothing || Base.invokelatest(hook, f) + return schedule(unscheduled(f)) +end # `f()` under `l`, for the cleanup sections, which have to get through; then `then`, if # given, on its value, outside the lock. The wait for the lock is where an exception @@ -1085,8 +1178,8 @@ end macro sslcall(ssl, op, expr) # the temporaries are gensyms: the whole quote is escaped, and plain names would # clobber a caller's locals of the same name - _err, _ret, _r, _e, _pending, _ended = - gensym.(("err", "ret", "r", "e", "pending", "ended")) + _err, _ret, _r, _e, _pending, _ended, _data = + gensym.(("err", "ret", "r", "e", "pending", "ended", "data")) esc(quote local $_err = nothing local $_ret = SSL_ERROR_NONE @@ -1098,8 +1191,17 @@ macro sslcall(ssl, op, expr) $ssl.closed && throwio($op) # clear the current error queue before openssl ccall clear_errors!() - # do the ccall - $_r = $expr + # do the ccall, the write BIO told to buffer what it produces for `take!` + # below (see `incall`); the flag a plain field store, which cannot fail. The + # result declared outside the `try`, which is a scope of its own + local $_r + local $_data = getfield($ssl, :data) + try + $_data.incall = true + $_r = $expr + finally + $_data.incall = false + end # the rest in a `try`: cut short (an interrupt at an allocation) after the # call, what the call left in the buffer is not to go out with another call's, # nor the stream to stay open. The records of a call that did not fail are @@ -1172,13 +1274,31 @@ function finish_sslcall!(ssl::SSLStream, ret::SSLErrorCode, pending, err; detach # failed: the stream was closed and aborted in `@sslcall` already err === nothing || throw(err) if detach && pending !== nothing - background() do - try - drain!(ssl, pending) - catch ex - # `drain!` closed the stream; the reader finds that out on its next call - @guarded @debug "SSL reply to the peer not sent" ex + try + background() do + try + drain!(ssl, pending) + catch ex + # `drain!` closed the stream; the reader finds that out on its next call + @guarded @debug "SSL reply to the peer not sent" ex + end + end + catch + # no task to send it from (none could be made, or scheduled: an allocation + # failed, an interrupt landed): the record is not going out, so its ticket is + # given up, which refuses every ticket after it rather than parking them on + # this one for good, and the stream is aborted, as for a write cancelled while + # it waited for its turn (see `drain!`); both of them, with an interrupt held + # back. Should the task have been scheduled all the same (the interrupt + # landing after that), its drain finds the ticket given up and does no harm + holdinterrupts() do + try + abandon!(getfield(ssl, :data), pending.ticket) + finally + close(ssl, false) + end end + rethrow() end else drain!(ssl, pending) @@ -1435,12 +1555,7 @@ function handshake!(step, ssl::SSLStream, op::Symbol, timeout::Real; finish=noth # another task. Or the deadline passed with the stream somehow still open (its # abort having failed): the deadline decides, not what the abort left (ssl.closed || state[] === :timedout) && throwio(op) - ccall( - (:SSL_set_read_ahead, libssl), - Cvoid, - (SSL, Cint), - ssl.ssl, - Cint(1)) + readahead!(ssl.ssl) end catch ex # whatever ended the handshake, an interrupt included, a half-done stream is of @@ -1479,21 +1594,25 @@ function hostname!(ssl::SSLStream, host) end """ - Sockets.accept(ssl::SSLStream; timeout=Inf) - -Runs the server side of the TLS handshake on `ssl` and returns once it is complete. -Throws `EOFError` when the peer goes away first, `IOError` when the handshake fails, -and `IOError` once `timeout` seconds have passed since the call began, however far the -handshake got; the stream is closed in each of those cases. `timeout` is a positive -number of seconds, or `Inf` for none. - -Before 1.6.2 the call did one round of the handshake and threw `OpenSSLError` whenever -it needed more bytes from the peer, and the caller retried. Those loops still work, they -get the completed handshake on the first call, but they no longer see `OpenSSLError`: -the errors are `EOFError` and `IOError` now. A deadline such a loop enforced between -attempts is what `timeout` is for. + Sockets.accept(ssl::SSLStream; timeout=nothing) + +Runs the server side of the TLS handshake on `ssl`. + +With `timeout`, a positive number of seconds or `Inf` for no limit, the whole of it: +returns once the handshake is complete; throws `EOFError` when the peer goes away first, +`IOError` when the handshake fails, and `IOError` once `timeout` seconds have passed +since the call began, however far the handshake got; the stream is closed in each of +those cases. + +Without `timeout`, one round of it, the contract the call has had since before 1.6.2: +returns once the handshake is complete, and throws `OpenSSLError` when it needs more +bytes from the peer first, for the caller to wait for them (`eof(ssl.io)`, say) and +call again; the loops written to that, which enforce their own deadline between calls, +keep working. A round that failed, the peer's certificate rejected say, throws `IOError` +and closes the stream, as every SSL call does. """ -function Sockets.accept(ssl::SSLStream; timeout::Real=Inf) +function Sockets.accept(ssl::SSLStream; timeout::Union{Nothing, Real}=nothing) + timeout === nothing && return acceptround!(ssl) handshake!(ssl, :accept, timeout) do @geterror ssl :accept ccall( (:SSL_accept, libssl), @@ -1504,6 +1623,28 @@ function Sockets.accept(ssl::SSLStream; timeout::Real=Inf) return end +# one round of the server side of the handshake, what the round produced sent before it +# returns; a round that needs more bytes says so with `OpenSSLError` and does not wait +# for them: the loops written to this call wait between calls, and enforce their +# deadlines there, which a call that waited for the peer itself would defeat +function acceptround!(ssl::SSLStream) + ret = @geterror ssl :accept ccall( + (:SSL_accept, libssl), + Cint, + (SSL,), + ssl.ssl) + if ret == SSL_ERROR_NONE + # done: read ahead set, as `handshake!` sets it + Base.@lock ssl.lock begin + ssl.closed && throwio(:accept) + readahead!(ssl.ssl) + end + return + end + # WANT_READ; WANT_WRITE cannot happen, the write BIO takes everything + throw(OpenSSLError("accept: $ret, the handshake needs more bytes from the peer; call again once there are some")) +end + """ Read from the SSL stream. """ @@ -1836,7 +1977,15 @@ function closerest!(ssl::SSLStream, shutdown::Bool) # costs the close_notify, and the close aborts data = getfield(ssl, :data) Base.@lock data.cond begin - data.lostfrom == typemax(Int) && ssl_disconnect(ssl.ssl) + if data.lostfrom == typemax(Int) + # buffered, for the `take!` below (see `incall`) + try + data.incall = true + ssl_disconnect(ssl.ssl) + finally + data.incall = false + end + end end end finally diff --git a/test/runtests.jl b/test/runtests.jl index 3764682..e620fd9 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -670,7 +670,7 @@ end function connected_pair(server_ctx, server) server_task = @async begin ssl = OpenSSL.SSLStream(server_ctx, accept(server)) - Sockets.accept(ssl) + Sockets.accept(ssl; timeout=Inf) ssl end port = Sockets.getsockname(server)[2] @@ -770,9 +770,10 @@ end # the kernel's send and receive buffers grow to (Windows and macOS keep growing them for # a peer that does not read): a parked write that the buffers could still take would # complete of itself, and the tests need it to stay parked until they end it +const PARK_CHUNK = max(32 * 2^20, 3 * OpenSSL.SSL_WRITE_CHUNK) function park_writer(client, stop_writing) written = Threads.Atomic{Int}(0) - chunk = zeros(UInt8, max(32 * 2^20, 3 * OpenSSL.SSL_WRITE_CHUNK)) + chunk = zeros(UInt8, PARK_CHUNK) writer = @async try while !stop_writing[] write(client, chunk) @@ -977,7 +978,7 @@ end server_task = @async begin ssl = OpenSSL.SSLStream(server_ctx, accept(server)) try - Sockets.accept(ssl) + Sockets.accept(ssl; timeout=Inf) catch end close(ssl) @@ -1331,6 +1332,198 @@ end close(server) end +# a client stream connecting to `port` on a task, for a server that drives its side of +# the handshake itself +function connecting(port) + @async begin + client = OpenSSL.SSLStream(OpenSSL.SSLContext(OpenSSL.TLSClientMethod(), ""), + Sockets.connect(ip"127.0.0.1", port)) + Sockets.connect(client; require_ssl_verification=false) + client + end +end + +# the loop written to `accept`'s contract of old: one round per call, `OpenSSLError` +# when it needs more bytes, the wait for them and the deadline between calls. Returns +# the outcome and how many rounds asked for more +function acceptloop(ssl, deadline) + rounds = 0 + while true + try + Sockets.accept(ssl) + return :ok, rounds + catch ex + ex isa OpenSSL.OpenSSLError || rethrow() + rounds += 1 + time() < deadline || return :deadline_expired, rounds + # waits for bytes as such a loop does, with `eof(ssl.io)`, which is what + # starts the socket reading; but not past the deadline, so on a task, which + # a peer that stays silent leaves waiting until the socket closes + arrival = @async eof(ssl.io) + timedwait(() -> istaskdone(arrival), max(deadline - time(), 0.01)) + end + end +end + +@testset "LegacyAccept" begin + # `accept` without `timeout` keeps the contract it had before 1.6.2: one round, which + # does not wait for the peer, so the deadline a loop checks between calls holds + # against a peer that stays silent; and the loop completes the handshake with one + # that talks + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + silent = Sockets.connect(ip"127.0.0.1", port) + ssl = OpenSSL.SSLStream(server_ctx, accept(server)) + outcome, rounds = acceptloop(ssl, time() + 0.5) + @test outcome === :deadline_expired + @test rounds >= 1 + # nothing failed: the loop could have gone on + @test isopen(ssl) + close(ssl) + close(silent) + + client_task = connecting(port) + ssl = OpenSSL.SSLStream(server_ctx, accept(server)) + outcome, rounds = acceptloop(ssl, time() + 30.0) + @test outcome === :ok + # the client's Finished is never in on the first round + @test rounds >= 1 + connected = timedwait(() -> istaskdone(client_task), 30.0) === :ok + @test connected + if connected && outcome === :ok + client = fetch(client_task) + write(client, UInt8[1, 2, 3]) + @test read!(ssl, Vector{UInt8}(undef, 3)) == UInt8[1, 2, 3] + write(ssl, UInt8[4]) + @test read!(client, Vector{UInt8}(undef, 1)) == UInt8[4] + close(client) + else + # a client still in its handshake is released by the abort + close(ssl, false) + end + close(ssl) + close(server) +end + +@testset "RawSSLCalls" begin + # a call made on `ssl.ssl` from outside the package has what it produces written to + # the socket by the write BIO callback itself, as before 1.6.2: an `SSL_accept` loop + # of a caller's own (`ssl_accept`, under `ssl.lock`, as TLSStreams 0.2 runs it) + # completes the handshake; and a raw write goes out behind the records the package's + # own calls ticketed, so the peer reads them in order + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + client_task = connecting(port) + ssl = OpenSSL.SSLStream(server_ctx, accept(server)) + deadline = time() + 30.0 + while true + done = Base.@lock ssl.lock begin + try + OpenSSL.ssl_accept(ssl.ssl) + true + catch ex + ex isa OpenSSL.OpenSSLError || rethrow() + false + end + end + done && break + time() < deadline || error("the raw accept loop did not finish") + eof(ssl.io) && error("the peer went away during the raw accept loop") + end + client = fetch(client_task) + write(client, UInt8[1, 2, 3]) + @test read!(ssl, Vector{UInt8}(undef, 3)) == UInt8[1, 2, 3] + + # the server reads nothing for now, so the client's writer parks; then a raw write on + # the client waits, under `ssl.lock`, for the parked record to be through, and its + # record goes out in the order OpenSSL made it in, whole. The parked write has + # records still to make when the raw one is made, and the raw caller holds + # `ssl.lock`, not the writers' lock: the raw record lands among that write's, which + # the peer reads as what each is, nothing lost and no record out of sequence + stop_writing = Threads.Atomic{Bool}(false) + parked_writer, written = park_writer(client, stop_writing) + stop_writing[] = true + # the completed writes and the parked one, all zeros, and the marker + nzeros = written[] + PARK_CHUNK + marker = fill(UInt8(7), 100) + raw = @async Base.@lock client.lock begin + n = Ref{Csize_t}(0) + r = GC.@preserve marker ccall( + (:SSL_write_ex, OpenSSL.libssl), + Cint, + (OpenSSL.SSL, Ptr{Cvoid}, Csize_t, Ptr{Csize_t}), + client.ssl, pointer(marker), length(marker), n) + (r, Int(n[])) + end + sleep(0.5) + @test !istaskdone(raw) + reader = @async read!(ssl, Vector{UInt8}(undef, nzeros + length(marker))) + @test timedwait(() -> istaskdone(reader), 60.0) === :ok + if istaskdone(reader) + received = fetch(reader) + @test count(==(0), received) == nzeros + sevens = findall(==(7), received) + @test length(sevens) == length(marker) + # in one piece + @test !isempty(sevens) && sevens == sevens[1]:sevens[1] + length(marker) - 1 + end + @test timedwait(() -> istaskdone(raw) && istaskdone(parked_writer), 30.0) === :ok + @test istaskdone(raw) && fetch(raw) == (1, 100) + @test istaskdone(parked_writer) && fetch(parked_writer) === nothing + close(client) + close(ssl) + close(server) +end + +@testset "DetachedDrainNotScheduled" begin + # what a read produces for the peer goes out from a task of its own (see + # `finish_sslcall!`). When that task cannot be made or scheduled, the record's ticket + # is given up and the stream aborted, so that the writers after it fail rather than + # wait for its turn for good. Driven directly: on current OpenSSL the one such reply, + # to a KeyUpdate, goes out with the next write instead, so no traffic reaches this + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + client, ssl = connected_pair(server_ctx, server) + data = client.data + # a record a read left in the write BIO, and its ticket, as `@sslcall` takes them; + # never sent, so its bytes do not matter + pending = Base.@lock client.lock begin + append!(data.buf, UInt8[0x17, 0x03, 0x03, 0x00, 0x01, 0x00]) + take!(data) + end + @test pending isa OpenSSL.PendingWrite + # only the reply's task fails to be made: the closure `finish_sslcall!` hands + # `background` is told by what it captures, `pending`; the watches and cleanups go on + OpenSSL.ONBACKGROUND[] = f -> hasproperty(f, :pending) && throw(OutOfMemoryError()) + try + err = try + OpenSSL.finish_sslcall!(client, OpenSSL.SSL_ERROR_NONE, pending, nothing; detach=true) + nothing + catch ex + ex + end + @test err isa OutOfMemoryError + @test !isopen(client) + # the ticket was consumed, and nothing after it may be sent + @test Base.@lock(data.cond, OpenSSL.drained(data)) + @test data.lostfrom == pending.ticket + # a write after it fails at once, not parked on the ticket that was given up + writer = @async try + write(client, UInt8[2]) + catch ex + ex + end + @test timedwait(() -> istaskdone(writer), 10.0) === :ok + @test istaskdone(writer) && fetch(writer) isa Base.IOError + @test timedwait(() -> !isopen(client.io), 30.0) === :ok + finally + OpenSSL.ONBACKGROUND[] = nothing + end + close(client, false) + close(ssl) + close(server) +end + @testset "SpareBuffer" begin # a chunk that reached the socket becomes the write BIO's next buffer; the peer # still reads every write whole and in order, across sizes around a record's and a From f82a40773a6e845c4f26cd2f187d5097376836c5 Mon Sep 17 00:00:00 2001 From: krynju Date: Fri, 2 Oct 2026 17:19:21 +0200 Subject: [PATCH 22/25] test: RawSSLCalls asserts the raw write waits only while the parked record is pending Windows grows the socket buffers and may let the parked record through within the half second, after which the raw write is free to go; the ticket turn tells whether it is still pending. Read after istaskdone, so a done write with the turn still on the record is the only failure. Co-Authored-By: Claude Fable 5.1 --- test/runtests.jl | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/runtests.jl b/test/runtests.jl index e620fd9..f18fbe5 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -1446,6 +1446,8 @@ end # the completed writes and the parked one, all zeros, and the marker nzeros = written[] + PARK_CHUNK marker = fill(UInt8(7), 100) + # the turn is on the parked record for as long as it is pending + turn0 = Base.@lock client.data.cond client.data.turn raw = @async Base.@lock client.lock begin n = Ref{Csize_t}(0) r = GC.@preserve marker ccall( @@ -1456,7 +1458,14 @@ end (r, Int(n[])) end sleep(0.5) - @test !istaskdone(raw) + # the raw write waits while the parked record is pending. Windows grows the socket + # buffers meanwhile and may let that record through, after which the raw write is + # free to go: the turn tells which. Read in this order: a raw write that is done + # had the turn move first, so a done write with the turn still on the record would + # be the fault, and nothing else is + rawdone = istaskdone(raw) + stillparked = Base.@lock client.data.cond client.data.turn == turn0 + @test !(rawdone && stillparked) reader = @async read!(ssl, Vector{UInt8}(undef, nzeros + length(marker))) @test timedwait(() -> istaskdone(reader), 60.0) === :ok if istaskdone(reader) From bfff910361c98ea0acafbbce49855d615203e7f0 Mon Sep 17 00:00:00 2001 From: krynju Date: Fri, 2 Oct 2026 17:41:37 +0200 Subject: [PATCH 23/25] test: skip the in-flight writer cancellations on Windows CancelledInFlightWriter and ThrowingLogger cancel a writer inside the socket write. Base's uv_writecb_task schedules the waiting task unconditionally while the request still names it, so a write completing just as its task is cancelled throws "schedule: Task not runnable" out of the libuv callback into the task running the event loop, and can wedge the loop. Windows grows the socket buffers for a peer that does not read, so a parked write completes on its own there and hits that window often (seen hanging two CI jobs). Co-Authored-By: Claude Fable 5.1 --- test/runtests.jl | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/runtests.jl b/test/runtests.jl index f18fbe5..1fbae70 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -1219,6 +1219,17 @@ end end @testset "CancelledInFlightWriter" begin + # Not on Windows: these cancel a writer inside the socket write, and Base's write + # completion callback (`uv_writecb_task`) schedules the waiting task unconditionally + # while the request still names it, which it does until the task resumes. A write + # that completes just as its task is cancelled therefore throws "schedule: Task not + # runnable" out of the libuv callback, into whatever task runs the event loop, and + # can wedge the loop. Windows grows the socket buffers for a peer that does not read, + # so a parked write completes on its own there and hits that window often; nothing + # this package can do about it +if Sys.iswindows() + @test_skip !Sys.iswindows() +else # A writer cancelled inside the socket write itself, not while waiting for its turn: # libuv still holds the write, and a pointer into the chunk, so the socket has to be # closed outright, at once; a normal close would wait behind that very write. @@ -1301,6 +1312,7 @@ end close(ssl) close(server) end +end @testset "WriteCount" begin # the closing bit and the count of writes in flight share one word; the bit is the @@ -2288,6 +2300,10 @@ end saw(logger::ThrowingLogger, what) = Base.@lock logger.lock any(m -> occursin(what, m), logger.seen) @testset "ThrowingLogger" begin + # Not on Windows: see CancelledInFlightWriter +if Sys.iswindows() + @test_skip !Sys.iswindows() +else # what the library logs on its cleanup paths cannot stop them: a cancelled in-flight # write through the fallback close, which logs before it schedules the close, with # the writer (and the tasks it starts) logging to a logger that throws @@ -2344,6 +2360,7 @@ saw(logger::ThrowingLogger, what) = Base.@lock logger.lock any(m -> occursin(wha close(ssl) close(server) end +end @testset "LostRecordNoCloseNotify" begin server_ctx = selfsigned_server_ctx() From 3bb045b1d1a96111e5616398ab73774fe4f49248 Mon Sep 17 00:00:00 2001 From: krynju Date: Sat, 3 Oct 2026 11:34:16 +0200 Subject: [PATCH 24/25] Bound the raw write's wait, cut only a write libuv may hold, fail with -1 Third review round, on the raw-call path and the unscheduled reply task: - awaitdrained gives up once the turn has not moved for CLOSE_GRACE. It waits under ssl.lock, which every close, abort and watch needs first, so a ticket no task will ever drain would otherwise hold the stream for good with nothing able to reach it. - rawwrite cuts the socket only for a write that was started and that libuv did not finish, the one case where a request may still point into OpenSSL's buffer. Any other failure just records the loss, so the records ticketed before it still go out with the close. An exception thrown into the raw caller's task is logged rather than dropped silently. - A failing write callback returns -1, not 0: OpenSSL up to 3.5.6 takes a zero with no retry flag as "nothing written yet", reports the call a success and keeps the record pending; -1 fails it in every version. The callback's catch-all returned 0 on main too. - background reports whether it scheduled the task, and finish_sslcall! gives the ticket up only when it did not: the drain and the catch passing one turn between them would carry it past the tickets. - CancelledInFlightWriter and ThrowingLogger run on Linux only; macOS grows the socket buffers as Windows does. The park_writer comment says which part of a parked write can still complete on its own. Test: RawWriteGivesUp. Co-Authored-By: Claude Fable 5.1 --- src/ssl.jl | 108 +++++++++++++++++++++++++++++++++++------------ test/runtests.jl | 73 ++++++++++++++++++++++++++------ 2 files changed, 142 insertions(+), 39 deletions(-) diff --git a/src/ssl.jl b/src/ssl.jl index 1103368..8f16545 100644 --- a/src/ssl.jl +++ b/src/ssl.jl @@ -356,40 +356,85 @@ drained(data::BIOStreamData) = data.turn == data.nextticket # writes a record made by a call from outside the package (see `incall`) to the socket, # from the write BIO callback, under whatever locks the caller holds, as before 1.6.2. # Once every record the package's own calls ticketed is through, so that the records -# keep the order OpenSSL made them in. Blocks while the peer's window is full, and -# nothing of the package's can bound that, every close and watch needing `ssl.lock` -# first: the caller bounds it as it did then, by closing the socket. Returns what the -# BIO callback does: the count, or 0 for a failure, which fails the SSL call. A failure -# leaves the stream unusable (OpenSSL takes a BIO failure as fatal), and a write that -# did not complete may be queued in libuv still, with a pointer into OpenSSL's own -# buffer, which `SSL_free` would free under it: so the socket is cut, which cancels the -# write, and the loss recorded, and nothing more goes out on this stream. An interrupt, -# which cannot be thrown through OpenSSL's frames, is said to be lost +# keep the order OpenSSL made them in (see `awaitdrained`, which also bounds the wait). +# The write itself blocks while the peer's window is full, and nothing of the package's +# can bound that, every close and watch needing `ssl.lock` first: the caller bounds it +# as it did then, by closing the socket. Returns what the BIO callback does: the count, +# or 0 for a failure, which fails the SSL call. A failure leaves the stream unusable +# (OpenSSL counted the record, and takes a BIO failure as fatal), so the loss is +# recorded and nothing more goes out on the stream; the records ticketed before it +# still go, with the close that follows. A write that was started and that libuv did +# not finish may be queued still, with a pointer into OpenSSL's own buffer, which +# `SSL_free` would free under it: then the socket is cut, which cancels the write. +# Nothing can be thrown through OpenSSL's frames: an interrupt is said to be lost, and +# anything else thrown into the caller's task (a cancellation) is logged, the call +# failing in its place. The failure is -1 (see `on_bio_stream_write`) function rawwrite(data::BIOStreamData, in::Ptr{Cchar}, inlen::Cint)::Cint written = 0 sent = false + started = false + finished = false try if awaitdrained(data) + started = true written = unsafe_write(data.io, in, inlen) sent = true end catch ex - ex isa InterruptException && lostinterrupt() + # the write ended with libuv's own error status for it (see `drain!`): the + # request is finished, and nothing of it queued + finished = ex isa Base.IOError && ex.code < 0 && startswith(ex.msg, "write:") + if ex isa InterruptException + lostinterrupt() + elseif finished + @guarded @debug "OpenSSL: the socket write of a raw SSL call failed" exception=caught(ex) + else + @guarded @warn "OpenSSL: an exception landed in the socket write of a raw SSL call and could not be passed on; the call fails instead" exception=caught(ex) + end finally - sent || holdinterrupts(() -> cut!(data)) + sent || holdinterrupts() do + if started && !finished + cut!(data) + else + surely(() -> lose!(data, data.nextticket), data.cond) + end + end end - return sent ? Cint(written) : Cint(0) + return sent ? Cint(written) : Cint(-1) end # waits until every ticket handed out is through, for a write that takes no ticket (see -# `incall`); false once nothing more may be sent on the socket, it having been cut or a -# record lost. Takes `cond`; the caller holds `ssl.lock`, which keeps `nextticket` still +# `rawwrite`); false once nothing more may be sent on the socket, it having been cut or +# a record lost, and false once the turn has not moved for `CLOSE_GRACE` seconds: the +# wait holds `ssl.lock` (the raw caller's), which every close, abort and watch needs +# first, so a ticket no task will ever drain (its owner stopped between taking it and +# draining it) would hold the stream for good with nothing able to reach it; the +# graceful close's measure of a stall bounds it instead. Takes `cond`; the caller holds +# `ssl.lock`, which keeps `nextticket` still function awaitdrained(data::BIOStreamData) + grace = CLOSE_GRACE[] Base.@lock data.cond begin - while true - (data.cut || data.lostfrom != typemax(Int)) && return false - drained(data) && return true - wait(data.cond) + turn = data.turn + since = time_ns() + # wakes the wait at the deadline, a condition's `wait` having no bound of its own; + # a tick that comes once the timer is closed only notifies for nothing + timer = ticker(grace; interval=grace) do _ + Base.@lock data.cond notify(data.cond) + end + try + while true + (data.cut || data.lostfrom != typemax(Int)) && return false + drained(data) && return true + if data.turn != turn + turn = data.turn + since = time_ns() + elseif (time_ns() - since) / 1e9 >= grace + return false + end + wait(data.cond) + end + finally + close(timer) end end end @@ -432,8 +477,11 @@ function on_bio_stream_write(bio::BIO, in::Ptr{Cchar}, inlen::Cint)::Cint written = unsafe_write(data::IO, in, inlen) return Cint(written) catch e - # we don't want to throw a Julia exception from a C callback - return Cint(0) + # we don't want to throw a Julia exception from a C callback. -1, not 0: OpenSSL + # up to 3.5.6 takes a zero from a write callback with no retry flag as a write + # that did nothing yet, reports the call a success and keeps the record pending + # for the next call to send; -1 fails the call in every version + return Cint(-1) end end @@ -880,10 +928,16 @@ end # the tests' hook (see `ONKEEP`): called with `f` before the task is made, and what it # throws stands for a failure to make or schedule the task const ONBACKGROUND = Ref{Any}(nothing) -function background(f) +# `scheduled`, if given, is set just before the task is scheduled, for a caller that has +# to know whether `f` is going to run should this throw (an interrupt landing after the +# fact); set before rather than after, so that a task that does run is never taken for +# one that does not +function background(f, scheduled::Union{Nothing, Base.RefValue{Bool}}=nothing) hook = ONBACKGROUND[] hook === nothing || Base.invokelatest(hook, f) - return schedule(unscheduled(f)) + task = unscheduled(f) + scheduled === nothing || (scheduled[] = true) + return schedule(task) end # `f()` under `l`, for the cleanup sections, which have to get through; then `then`, if @@ -1274,8 +1328,9 @@ function finish_sslcall!(ssl::SSLStream, ret::SSLErrorCode, pending, err; detach # failed: the stream was closed and aborted in `@sslcall` already err === nothing || throw(err) if detach && pending !== nothing + scheduled = Ref(false) try - background() do + background(scheduled) do try drain!(ssl, pending) catch ex @@ -1289,11 +1344,12 @@ function finish_sslcall!(ssl::SSLStream, ret::SSLErrorCode, pending, err; detach # given up, which refuses every ticket after it rather than parking them on # this one for good, and the stream is aborted, as for a write cancelled while # it waited for its turn (see `drain!`); both of them, with an interrupt held - # back. Should the task have been scheduled all the same (the interrupt - # landing after that), its drain finds the ticket given up and does no harm + # back. Not the ticket when the task was scheduled all the same (an interrupt + # landing after that): its drain sends the record or gives the ticket up + # itself, and two parties passing one turn would carry it past the tickets holdinterrupts() do try - abandon!(getfield(ssl, :data), pending.ticket) + scheduled[] || abandon!(getfield(ssl, :data), pending.ticket) finally close(ssl, false) end diff --git a/test/runtests.jl b/test/runtests.jl index 1fbae70..05c8bab 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -766,10 +766,12 @@ end # writes until the peer's receive window is full and the writer parks on the socket; # returns the task and the count of completed writes. Each write is many times what goes -# to OpenSSL in one call, so that the parked one has chunks still to go, and more than -# the kernel's send and receive buffers grow to (Windows and macOS keep growing them for -# a peer that does not read): a parked write that the buffers could still take would -# complete of itself, and the tests need it to stay parked until they end it +# to OpenSSL in one call, so that the parked one has records still to make, and more +# than the kernel's send and receive buffers grow to (Windows and macOS keep growing +# them for a peer that does not read): a parked write that the buffers could still take +# would complete of itself, and the tests need it to stay parked until they end it. One +# record of it, the one in libuv, may still be accepted as the buffers grow, which is +# all the Base race described at CancelledInFlightWriter needs const PARK_CHUNK = max(32 * 2^20, 3 * OpenSSL.SSL_WRITE_CHUNK) function park_writer(client, stop_writing) written = Threads.Atomic{Int}(0) @@ -1219,16 +1221,19 @@ end end @testset "CancelledInFlightWriter" begin - # Not on Windows: these cancel a writer inside the socket write, and Base's write + # Linux only: these cancel a writer inside the socket write, and Base's write # completion callback (`uv_writecb_task`) schedules the waiting task unconditionally # while the request still names it, which it does until the task resumes. A write # that completes just as its task is cancelled therefore throws "schedule: Task not # runnable" out of the libuv callback, into whatever task runs the event loop, and - # can wedge the loop. Windows grows the socket buffers for a peer that does not read, - # so a parked write completes on its own there and hits that window often; nothing - # this package can do about it -if Sys.iswindows() - @test_skip !Sys.iswindows() + # can wedge the loop. Windows and macOS grow the socket buffers for a peer that does + # not read, so the parked record (not the whole write, see `park_writer`) completes + # on its own there and hits that window, Windows often; on Linux it completes only + # when the peer reads, which these tests' peers never do. Nothing this package can do + # about it: the cancelled write's own cleanup here is sound, the moment of + # cancellation is Base's +if !Sys.islinux() + @test_skip Sys.islinux() else # A writer cancelled inside the socket write itself, not while waiting for its turn: # libuv still holds the write, and a pointer into the chunk, so the socket has to be @@ -1496,6 +1501,48 @@ end close(server) end +@testset "RawWriteGivesUp" begin + # a raw write waits for the records ticketed before it. One whose ticket no task will + # ever drain would hold it for good, under `ssl.lock`, where no close or watch can + # reach it: once nothing has moved for CLOSE_GRACE the wait gives up, the call fails, + # the loss is recorded so nothing more goes out, and the socket is not cut, no write + # having been started + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + client, ssl = connected_pair(server_ctx, server) + data = client.data + # a ticket taken and never drained + stale = Base.@lock client.lock begin + append!(data.buf, UInt8[0x17, 0x03, 0x03, 0x00, 0x01, 0x00]) + take!(data) + end + grace = OpenSSL.CLOSE_GRACE[] + OpenSSL.CLOSE_GRACE[] = 1.0 + try + marker = fill(UInt8(7), 10) + raw = @async Base.@lock client.lock begin + n = Ref{Csize_t}(0) + GC.@preserve marker ccall( + (:SSL_write_ex, OpenSSL.libssl), + Cint, + (OpenSSL.SSL, Ptr{Cvoid}, Csize_t, Ptr{Csize_t}), + client.ssl, pointer(marker), length(marker), n) + end + @test timedwait(() -> istaskdone(raw), 10.0) === :ok + @test istaskdone(raw) && fetch(raw) != 1 + @test data.lostfrom == stale.ticket + 1 + @test Base.@lock(data.cond, !data.cut) + # the stream is unusable from here + @test_throws Base.IOError write(client, UInt8[1]) + @test !isopen(client) + finally + OpenSSL.CLOSE_GRACE[] = grace + end + close(client, false) + close(ssl) + close(server) +end + @testset "DetachedDrainNotScheduled" begin # what a read produces for the peer goes out from a task of its own (see # `finish_sslcall!`). When that task cannot be made or scheduled, the record's ticket @@ -2300,9 +2347,9 @@ end saw(logger::ThrowingLogger, what) = Base.@lock logger.lock any(m -> occursin(what, m), logger.seen) @testset "ThrowingLogger" begin - # Not on Windows: see CancelledInFlightWriter -if Sys.iswindows() - @test_skip !Sys.iswindows() + # Linux only: see CancelledInFlightWriter +if !Sys.islinux() + @test_skip Sys.islinux() else # what the library logs on its cleanup paths cannot stop them: a cancelled in-flight # write through the fallback close, which logs before it schedules the close, with From abe9d7a366fbf0b29e70bde97ece19bc002bb765 Mon Sep 17 00:00:00 2001 From: krynju Date: Sat, 3 Oct 2026 12:12:39 +0200 Subject: [PATCH 25/25] test: a task cancelled inside a raw write's socket write Linux only, as CancelledInFlightWriter. The cancellation is logged and the SSL call fails; the socket is cut, which cancels the request libuv may still hold into OpenSSL's buffer, and the loss is recorded. Co-Authored-By: Claude Fable 5.1 --- test/runtests.jl | 54 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/test/runtests.jl b/test/runtests.jl index 05c8bab..e236be3 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -1543,6 +1543,60 @@ end close(server) end +@testset "RawWriteCancelled" begin + # Linux only, as CancelledInFlightWriter: a task cancelled inside a raw write's socket + # write. Nothing can be thrown through OpenSSL's frames, so the cancellation is + # logged and the SSL call fails; the write libuv may still hold, with a pointer into + # OpenSSL's buffer, is cancelled by cutting the socket, and the loss is recorded +if !Sys.islinux() + @test_skip Sys.islinux() +else + server_ctx = selfsigned_server_ctx() + port, server = Sockets.listenany(ip"127.0.0.1", 20000) + client, ssl = connected_pair(server_ctx, server) + data = client.data + logger = Test.TestLogger() + # the server reads nothing: raw records fill the socket until one parks in its write + chunk = zeros(UInt8, 16 * 1024) + raw = Base.CoreLogging.with_logger(logger) do + @async Base.@lock client.lock begin + n = Ref{Csize_t}(0) + local r = Cint(1) + while r == 1 + r = GC.@preserve chunk ccall( + (:SSL_write_ex, OpenSSL.libssl), + Cint, + (OpenSSL.SSL, Ptr{Cvoid}, Csize_t, Ptr{Csize_t}), + client.ssl, pointer(chunk), length(chunk), n) + end + r + end + end + # parked: bytes queued in libuv, the same amount half a second later + queued() = OpenSSL.writequeuesize(client.io) + parked = timedwait(60.0; pollint=0.5) do + before = queued() + sleep(0.5) + !istaskdone(raw) && before > 0 && queued() == before + end + @test parked === :ok + if parked === :ok + schedule(raw, ErrorException("cancelled"); error=true) + @test timedwait(() -> istaskdone(raw), 10.0) === :ok + # the call failed, the task went on past it to return the failure + @test istaskdone(raw) && !istaskfailed(raw) && fetch(raw) != 1 + @test Base.@lock(data.cond, data.cut) + @test data.lostfrom != typemax(Int) + @test timedwait(() -> !isopen(client.io), 30.0) === :ok + @test any(l -> l.level == Base.CoreLogging.Warn && occursin("could not be passed on", l.message), logger.logs) + else + close(client, false) + end + close(ssl) + close(server) +end +end + @testset "DetachedDrainNotScheduled" begin # what a read produces for the peer goes out from a task of its own (see # `finish_sslcall!`). When that task cannot be made or scheduled, the record's ticket