Repository navigation
Conversation
Both ntcr::StreamSocket (reactor-based) and ntcp::StreamSocket
(proactor-based) previously serialized all operations on a single mutex,
so a call to 'send' or the processing of a socket writable/sent event
blocked a concurrent call to 'receive' or the processing of a socket
readable/received event, and vice-versa. Introduce a distinct mutex for
each direction so that the steady-state send and receive paths proceed
independently.
Locking strategy
----------------
Each stream socket now defines three mutexes, always acquired in the
canonical order 'd_mutex' (state), then 'd_sendMutex', then
'd_receiveMutex':
* 'd_sendMutex' guards the send queue, the send rate limiter and
deflater, the send deadline and rate timers, and the send-side
portion of an I/O completion.
* 'd_receiveMutex' guards the receive queue, the receive rate limiter
and inflater, the receive deadline and rate timers, and the
receive-side portion of an I/O completion.
* 'd_mutex' guards the socket lifecycle: opening, connecting,
upgrading, downgrading, shutting down, closing, detaching, and the
registration of managers, sessions, and resolvers.
Steady-state I/O holds only its own leaf mutex. When a completion
discovers a cross-cutting transition (end-of-stream, failure, or the
graceful shutdown sentinel), the drain functions now return that
condition to their caller rather than invoking the transition inline;
the caller releases the leaf mutex and re-acquires all three mutexes in
canonical order before performing the transition. Rare failures that
cannot conveniently return a status are deferred onto the reactor or
proactor via 'execute', so that they re-enter with no locks held.
Members that are read from more than one direction -- 'd_socket_sp',
'd_encryption_sp', 'd_session_sp', 'd_sessionStrand_sp', 'd_manager_sp',
and 'd_managerStrand_sp' -- are written only while holding all three
mutexes and may be read while holding any one of them.
Announcements and callback dispatches now name the mutex actually held
at the point of dispatch, so that a synchronous dispatch unlocks and
relocks the correct mutex. Any dispatch performed while holding more
than one mutex is forced to defer.
Encrypted sockets
-----------------
A TLS receive completion may need to write outgoing ciphertext, which
touches send-side state. The unencrypted path, which is the common case,
therefore takes only its own leaf mutex, while the encrypted path checks
for the presence of an encryption session under the leaf mutex, then
re-acquires all three mutexes in canonical order and repeats the
completion. Likewise, 'send' escalates to the state and send mutexes
only when the socket is encrypted.
Behavior preserved
------------------
Starting a new write still fails if the socket is not yet connected, is
shut down in the send direction, or is being shut down in the send
direction, while writes already queued still drain when the shutdown is
graceful. Starting a new read still fails once the socket is shut down
for reading and the receive queue has been fully dequeued.
Supporting components
---------------------
Lifecycle predicates must remain observable from a thread that holds
only a leaf mutex, so the state components consulted on the I/O paths
no longer require the state mutex to be read:
* ntcs::OpenState stores its value in a 'bsls::AtomicInt'.
* ntcs::DetachState stores its mode in a 'bsls::AtomicInt'.
* ntcs::ShutdownState maintains atomic mirrors of the predicates
derived from its context, refreshed whenever the context changes.
* ntcs::FlowControlState is now thread safe. Its five boolean data
members are packed into the bits of a single 'bsls::AtomicUint', and
'apply' and 'relax' commit their compound transitions with a
compare-and-swap retry loop. This allows a single flow control state
object to remain shared between the send and receive directions
rather than being split per-direction.
Tests
-----
Update the ntcr::StreamSocket test driver so that its reactor mock
expectations account for the cross-cutting announcements that are now
delivered deferred: connect completion and failure, upgrade completion
and failure, and error. The affected cases capture the functor passed
to 'execute' and invoke it before asserting. The ntcp::StreamSocket test
driver required no changes, as it exercises real sockets rather than
mocks.
…receive mutexes in stream sockets
Continue refining the locking strategy of ntcr::StreamSocket and
ntcp::StreamSocket so that sending, including sending encrypted data, is
never serialized against receiving, and so that the receive path never
needs to release its mutex to perform a cross-cutting transition.
Encrypted sends take only 'd_sendMutex'
---------------------------------------
Previously 'send' escalated to the state and send mutexes when the socket
was encrypted, on the grounds that the encryption session is shared with
the receive path. That escalation was unnecessary:
* The encryption session is itself thread safe.
* 'd_encryption_sp' is written only while holding every mutex, so it
is stable while 'd_sendMutex' is held.
* Every path that pops outgoing cipher text and enqueues it to the
send queue holds 'd_sendMutex', so TLS records are enqueued in the
order the session generates them, even when the receive path
generates cipher text concurrently (e.g., a TLS 1.3 key update).
* Pushing outgoing plain text never invokes the handshake callback
(see below), so the encrypted send path never performs a
cross-cutting transition.
Both 'send' overloads now take only 'd_sendMutex', the 'privateSend'
helpers that existed only to support the escalation are folded back into
'send', and 'privateSendEncrypted' again honors 'options.recurse()' as it
did before the lock split, removing a forced deferral of send completion.
Handshake callback contract
---------------------------
Document on ntci::Encryption that implementations invoke the handshake
callback only from within 'initiateHandshake' or
'pushIncomingCipherText', i.e., from the call that supplies the input
that determines the outcome of the handshake, and never from
'pushOutgoingPlainText', 'popIncomingPlainText', 'popOutgoingCipherText',
or 'shutdown'. The stream sockets rely on this guarantee to keep the
encrypted send path free of cross-cutting transitions. Both
implementations are made to honor it robustly:
* ntctls::Session clears the thread's OpenSSL error queue before
'SSL_do_handshake', 'SSL_write', 'SSL_read', and 'SSL_shutdown'.
Previously a stale, unrelated error on the calling thread could make
'SSL_get_error' misreport a pending handshake as failed, invoking the
handshake callback from an arbitrary call, including on the send
path. The failure path also no longer invokes an empty callback if
the failure has already been announced.
* ntcd::Encryption announces the outcome of the handshake only after
all complete incoming frames have been consumed. Previously it
released its mutex to invoke the callback while the frame being
processed was still in its buffer, so a concurrent caller could
observe and re-process that frame and receive a spurious error. The
mutex is now handed off to the callback and not re-acquired after it
returns, so the callback may safely destroy the encryption session.
The stream sockets also retain a reference to the encryption session
across 'initiateHandshake' and 'pushIncomingCipherText', since a failed
handshake resets 'd_encryption_sp' from within the callback and would
otherwise destroy the session while one of its member functions is still
executing. 'privateUpgrade' now stops if the callback has already
processed the failure of the upgrade, rather than dereferencing the
reset pointer.
Merge 'd_mutex' into 'd_receiveMutex'
------------------------------------
Each stream socket now defines two mutexes, always acquired in the order
'd_receiveMutex', then 'd_sendMutex':
* 'd_receiveMutex' guards the read path and the lifecycle and control
state of the socket (the connect and upgrade machinery, and the
transitions of the open, shutdown, and detach state machines).
Conceptually, it guards the connectedness and potential readability
of the socket.
* 'd_sendMutex' guards the separate write path.
This order follows the flow of data: the read path may feed the write
path, so it may acquire 'd_sendMutex' while holding 'd_receiveMutex',
but the write path never acquires 'd_receiveMutex' while holding
'd_sendMutex'. No code path acquired the former state mutex and then,
separately, the receive mutex, so the merge introduces no
self-deadlock. The only operations that now wait behind the read path
are opening, binding, connecting, registering resolvers, and the
certificate and private key accessors, none of which are on a hot path.
Function-level documentation that referred to the removed mutex now
names the mutex each function actually requires; several of these
contracts were already inaccurate after the original split.
Receive path performs cross-cutting transitions in place
--------------------------------------------------------
Because the receive path already holds 'd_receiveMutex', it now performs
every cross-cutting transition it discovers by additionally acquiring
'd_sendMutex', without releasing 'd_receiveMutex':
* Once the handshake has completed, decrypting incoming data requires
only 'd_receiveMutex': the handshake callback has been cleared and
can no longer be invoked. 'd_upgradeInProgress' selects this path.
While the handshake is in progress, the read holds both mutexes,
since it may complete or fail the handshake. Encrypted receive
callbacks may therefore again be invoked synchronously in the
steady state.
* Outgoing cipher text generated while decrypting is enqueued by
additionally acquiring 'd_sendMutex'.
* Receiving the peer's TLS shutdown, or unencrypted leftovers after
the encrypted data, completes the downgrade (which resets the
encryption session) by additionally acquiring 'd_sendMutex'. The
downgrade logic is factored into 'privateReceiveDowngrade', and the
cipher text drain into 'privateSendOutgoingCipherText'.
* End-of-file and failures are processed by additionally acquiring
'd_sendMutex', replacing the previous release-and-reacquire blocks.
Only the write path still releases its mutex and then acquires both
mutexes in order when it discovers a cross-cutting transition.
In ntcp::StreamSocket, the downgrade is now completed before the data
received is announced. Announcing the data may invoke receive callbacks
synchronously, which releases 'd_receiveMutex' while they execute;
previously a reception initiated concurrently during that window could
push unencrypted data into a session that had already received the
peer's TLS shutdown, failing the socket. Both stream sockets now order
the downgrade events before the data received in the same read.
Miscellaneous
-------------
The rate-limit announcements in 'privateThrottleSendBuffer' and
'privateThrottleReceiveBuffer' named the former state mutex. They are
deferred, so this was latent, but they now name 'd_sendMutex' and
'd_receiveMutex' respectively.
Testing
-------
ntcr_streamsocket.t, ntcp_streamsocket.t, ntcd_encryption.t,
ntcd_test.t, ntctls_plugin.t, the ntcr and ntcp datagram and listener
socket drivers, and ntcf_system.t (which exercises TLS upgrade, use,
shutdown, and abortive downgrade over ntcr::StreamSocket) pass on
Darwin. Darwin provides no proactor driver, so the encrypted paths of
ntcp::StreamSocket remain to be exercised by ntcf_system.t on Linux
(io_uring) or Windows (I/O completion ports).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is a work in progress.