refactor(sm): run dialed connections through the protocol core

Merged
GO
16d ago
+9068-3898

Step 4 of the plan adopted in #99. Every connection an sm.Client dials
or wraps with NewConn now has one diam/internal/protocol initiator
core, and that core makes the base-protocol decisions: building and
sending the CER, the retransmission budget and identifiers, what to do
with traffic while the CEA is awaited, CEA validation and correlation,
watchdog supervision, received DPR, Disconnect and Closing. The
Client's own receive loop, its CEA, DWA and DPR handlers, its separate
watchdog loop and its Disconnect bookkeeping are gone. So is the
StateMachine's closing-state map, which kept an entry forever for a
dialed Conn that offered no closure signal; such a Conn now keeps its
core in its own context, as accepted connections already do.

The executor that runs accepted connections now runs dialed ones too:
writes, hooks, logs, observers and lifecycle joins happen outside the
core's mutex, timers carry its epochs, native pending-request tickets
fence the CER and DWR writes, and every selected send gets a terminal
result.

Every Dial variant and NewConn keep their signatures, templates,
sentinel errors and callback timing.
Cancellation aborts the core before closing the transport and joins
the callback before returning. The first CER write keeps its
synchronous transport-error boundary, its TLS handshake and its write
deadline. A transport that closes before the handshake starts still
completes the call. A peer that closes after a successful CER write
without answering still leaves the response interval to run out, which
returns ErrHandshakeTimeout or, with retries left, the native error of
the next CER write; a core option keeps that response window for the
Client only. A duplicate CEA, malformed or not, is still reported as
an unexpected message and leaves the connection open.

A failed handshake returns detached copies of the core's evidence.
Joined errors, typed wrappers, mutable leaves and exported metadata
are copied with cycles cut and the work bounded, so changing what
errors.As extracts from the returned error cannot change the core's
record. Sentinels keep their identity, and
*smparser.ErrFailedResultCode keeps its full parsed CEA.

Behaviour changes on dialed connections, from the #109 rows assigned
to this step (RFC 3539 §3.4.1 and Appendix A, RFC 6733 §5.5):

  • Invalid or unowned input restarts Tw from its receipt time but no
    longer counts as recovery: a malformed or invalid repeated CER, other
    recoverable decode errors, an invalid header, a grammar, request or
    application validation failure, a DWR or DPR from the wrong host,
    and an unsolicited CEA or DPA.
  • An OnCER or OnDWR hook that changes the command code, application
    ID, request bit, Hop-by-Hop or End-to-End identifier of a dialed CER
    or DWR closes the connection with an Error record. Settings documents
    the rule.
  • A CER retry still queued behind the native write gate when the CEA
    arrives is cancelled: NewConn returns promptly and no later CER is
    sent.

Sharing the executor also fixes four things on accepted connections:

  • A DWA whose identifiers match a DWR that has not been written yet
    cancels that DWR instead of letting it go out after its answer. The
    DWA is still reported as invalid and earns no health credit, and the
    next Tw expiry selects a fresh probe.
  • A custom Conn whose Context returns nil no longer panics when the
    peer metadata is published.
  • Errors carried by logs and notifications are detached copies, as on
    the dialed side.
  • A CER received during Closing after the peer metadata was removed
    from the context is discarded: Closing and its deadline stay in
    force instead of a new handshake starting.

The executor also does less work per message: it compacts each batch's
effects in place, allocates observer slots only for effects that are
observed, builds the watchdog stop and arm as one pair and captures only
the timer kind and token in its callbacks. In BenchmarkInboundMessages
over TCP, accepted exchanges now allocate 43 to 46% fewer bytes and 27
to 70 fewer objects than before, and run 5 to 28% faster. Dialed
exchanges still go through the CloseNotify read bridge they already
used; behind it, the core's synchronous receive path makes them 52 to
77% slower than before with two processors, and up to 21% slower with
one. #134 removes the bridge.

Everything else stays as it was: the public API, Settings, hooks, log
records, OnProtocolError notifications, and the handling of connections
whose context already carries smpeer metadata. smparser hands its
validated-CEA conversion to sm through diam/sm/internal/parserbridge, so
its exported API is unchanged. peer.Manager keeps its own implementation
until #112.

Private tests that reached into the removed loop now drive the core or
the wire. New TCP tests cover the invalid-input matrix, the hook
coordinates, the queued CER retry, peer EOF while the CEA is awaited,
a transport that closes before the handshake starts, duplicate and
malformed CEAs, the isolation of returned errors, and the OnDWR hook an
OnHandshake callback installs for a DWR held behind it.

Fixes #111


Summary by cubic

Completes step 4 of the plan in #99 by running every sm.Client dialed connection through the shared diam/internal/protocol initiator core, so dialed and accepted connections use the same handshake, watchdog, and closing logic. The client’s own receive loop, CEA/DWA/DPR handlers, watchdog loop, and the state machine’s closing-state map are removed; the existing executor now serves both sides.

Behavior changes

  • Invalid or unowned input restarts Tw from receipt but no longer counts as recovery.
  • OnCER/OnDWR hooks that alter command code, app ID, request bit, or identifiers close the connection.
  • A CER retry queued behind the write gate is cancelled when the CEA arrives.
  • Accepted connections gain fixes: a DWA cancels an unwritten matching DWR; a nil context no longer panics; log/notification errors are detached copies; a CER during Closing is discarded.

Accepted connections allocate 43–46% fewer bytes and run 5–28% faster; dialed traffic is slower until #134 removes the read bridge. Public API, hooks, sentinel errors, and callback timing are unchanged. Fixes #111.