Skip to content

Disassociate the wrapped handle when a throttled association stops - #8647

Merged
Aaronontheweb merged 2 commits into
akkadotnet:devfrom
Aaronontheweb:fix/throttler-disassociate-wrapped-handle
Sep 25, 2026
Merged

Aaronontheweb merged 2 commits into
akkadotnet:devfrom
Aaronontheweb:fix/throttler-disassociate-wrapped-handle

Conversation

@Aaronontheweb

@Aaronontheweb Aaronontheweb commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

Closes #8637

Problem

ThrottlerHandle.Disassociate() only sent a PoisonPill to its ThrottledAssociation actor. That actor never disassociated the wrapped AssociationHandle, so with the trttl adapter applied (every MultiNodeSpec applies [trttl, gremlin]), the underlying TCP socket stayed open on both peers until the transport's Shutdown eventually force-closed it, instead of a graceful close.

Fix

ThrottledAssociation now disassociates the wrapped handle from PostStop (in a try/finally, so base.PostStop() always runs), so it happens exactly once no matter why the actor stopped (explicit disassociate, Blackhole, a crash, or transport shutdown). This mirrors canonical Akka's ThrottledAssociation.postStop, which does the same unconditional call and relies on the wrapped handle's Disassociate() being idempotent (a documented contract of AssociationHandle.Disassociate) - so there's no double-disassociate risk if the peer already tore the connection down first. Two call sites that used to disassociate the wrapped handle directly, and would now double up with PostStop, were removed.

Throttle mode behavior (Blackhole, TokenBucket) is otherwise unchanged - except that a new inbound association that gets blackholed during the handshake now closes its TCP socket as part of stopping, so the peer fails fast instead of waiting out the handshake timeout. This matches JVM Akka.

Testing

Added two tests to ThrottlerManagerLifecycleSpec, using its existing stub transport/handle harness, and strengthened an existing one:

  • explicit ThrottlerHandle.Disassociate() now tears down the wrapped handle
  • an inbound Disassociated notification (peer closed first) still disassociates the wrapped handle exactly once, via PostStop
  • the pre-existing "payload before handle" test now also asserts the wrapped handle is disassociated exactly once, guarding against the removed redundant call site regressing

All three fail on dev (or would, in the strengthened case) and pass with this change. Ran ThrottlerTransportAdapterSpec + the new/updated tests 5x, and the full Akka.Remote.Tests Transport namespace once - all green. Also ran a couple of MultiNode specs locally (AttemptSysMsgRedeliverySpec, RemoteNodeShutdownAndComesBackSpec) - both pass; no measurable termination-time change was observed locally, which is expected since DotNettyTransport.Shutdown (post-#8635) only waits on handles that already started a graceful close, so previously-unclosed throttled handles weren't being waited on either - this fix makes them close gracefully instead of via force-close, without changing the shutdown wait.

Test-infrastructure only: trttl isn't applied in default configs (applied-adapters = []).

…kkadotnet#8637)

ThrottlerHandle.Disassociate only PoisonPilled its ThrottledAssociation
actor, which never tore down the underlying TCP handle - sockets stayed
open under the trttl adapter (applied by every MultiNodeSpec) until
transport Shutdown force-closed them. ThrottledAssociation.PostStop now
disassociates the wrapped handle exactly once, matching canonical Akka's
postStop hook; the wrapped handle's Disassociate() is required to be
idempotent, so this is safe even after an inbound disassociation.

@Aaronontheweb Aaronontheweb left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Aaronontheweb
Aaronontheweb enabled auto-merge (squash) September 25, 2026 13:00
@Aaronontheweb
Aaronontheweb enabled auto-merge (squash) September 25, 2026 13:01
…associate, rename a misleading test

- ThrottledAssociation.PostStop: wrap the wrapped-handle Disassociate in
  try/finally so base.PostStop() (and OnTermination) always runs.
- Remove the now-redundant explicit OriginalHandle.Disassociate() in the
  "InboundPayload before initialized" handler - PostStop already covers it.
- Rename the inbound-disassociation test to describe what it actually
  checks (PostStop disassociates exactly once), and add a call-count
  assertion to the pre-existing "payload before handle" test so it would
  catch the removed redundant call coming back.
@Aaronontheweb
Aaronontheweb merged commit 1dca4b4 into akkadotnet:dev Sep 25, 2026
16 checks passed
Aaronontheweb added a commit to Aaronontheweb/akka.net that referenced this pull request Oct 2, 2026
…kkadotnet#8637) (akkadotnet#8647)

ThrottlerHandle.Disassociate only PoisonPilled its ThrottledAssociation
actor, which never tore down the underlying TCP handle - sockets stayed
open under the trttl adapter (applied by every MultiNodeSpec) until
transport Shutdown force-closed them. ThrottledAssociation.PostStop now
disassociates the wrapped handle exactly once; the wrapped handle's Disassociate() is required to be
idempotent, so this is safe even after an inbound disassociation.

(cherry picked from commit 1dca4b4)
Aaronontheweb added a commit to Aaronontheweb/akka.net that referenced this pull request Oct 3, 2026
…kkadotnet#8637) (akkadotnet#8647)

ThrottlerHandle.Disassociate only PoisonPilled its ThrottledAssociation
actor, which never tore down the underlying TCP handle - sockets stayed
open under the trttl adapter (applied by every MultiNodeSpec) until
transport Shutdown force-closed them. ThrottledAssociation.PostStop now
disassociates the wrapped handle exactly once; the wrapped handle's Disassociate() is required to be
idempotent, so this is safe even after an inbound disassociation.

(cherry picked from commit 1dca4b4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Throttler transport adapter never disassociates the underlying TCP handle

1 participant