Skip to content

Add two-phase DTLS session shutdown - #67

Merged
QuiteYellow merged 1 commit into
QuiteYellow:mainfrom
Moballo-LLC:codex/two-phase-session-shutdown
Aug 29, 2026
Merged

Add two-phase DTLS session shutdown#67
QuiteYellow merged 1 commit into
QuiteYellow:mainfrom
Moballo-LLC:codex/two-phase-session-shutdown

Conversation

@Jason-Morcos

@Jason-Morcos Jason-Morcos commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Stack order: #67#68#69. This is 1 of 3 and should merge first.

Home Assistant begins integration shutdown before its blocking worker pool has necessarily drained. Closing the UDP socket at that point wakes blocked protocol work, but it also discards the established DTLS state before close_notify can be sent. On Samsung appliances that accept only one local client, the abandoned association can then remain in the way until the appliance expires it.

This change gives callers a two-phase shutdown contract:

  • quiesce_for_close() stops new work and wakes the handshake, request, Observe-pacing, and refetch waiters while retaining an established DTLS transport.
  • close() performs an orderly close-notify drain once the caller has joined its workers.
  • abort() remains available when the transport must be torn down immediately.
  • Handshake publication and reader startup are synchronized with concurrent shutdown, and a failed subscription rolls back its local registration state.

The lifecycle cancellation is owned by the session. A caller-provided cancellation event remains an input and is not mutated as a shutdown side effect.

This is intentionally limited to transport lifecycle. It does not change authentication, discovery, request retransmission, capability handling, or integration worker scheduling. It implements the shutdown boundary discussed in #28 so downstream integrations can stop blocking work before releasing the DTLS association.

Validation at d33f896d84259a8ac6bcaaaad369bbfbd4c52c75:

  • 83 focused lifecycle, session, and Observe tests
  • 538 full SmartThings-Local tests on both the project interpreter and Python 3.11
  • 1,730 LocalThings tests with this head loaded
  • clean wheel and sdist builds with verified distribution contents

@QuiteYellow

Copy link
Copy Markdown
Owner

Ran the stack here: 538 passed on this head, matching your number.

The close_notify drain is the same fix I have sitting uncommitted on a branch
here. This one is better, so I'm dropping mine.

The measured gain on my hardware, fixed source port, previous session dropped
without close_notify against closed with it:

device orphan notify
dryer 2.0 / 1.7 / 2.0 s 0.7 / 0.8 / 0.8 s
oven 4.9 s 1.7 s

An orphan costs one swallowed ClientHello, so one OpenSSL retransmit timer.
close_notify removes it.

Taking this as it stands. One thing for a follow-up: the two-phase path sends
no OBSERVE deregisters.

quiesce_for_close() sets _lifecycle_cancel and _send_dgram then raises, so
the dereg sweep at the top of close() is skipped. Plain close() still sends
them. Against a scripted session here, one dereg for close() and none for
quiesce_for_close() followed by close().

close_notify does not cover the same ground. In the RT-OCF tree I have checked
out, the observer table in rt_coap_observe.c is keyed by
(addr, port, uri/token) and sits apart from the DTLS peer list, and the two
teardown calls that would clear it are both commented out:
rt_coap_transactions.c:130 and rt_coap_engine.c:205. On that reading the
explicit deregister is the only thing that removes an entry. Two caveats: that
tree was last pushed in 2023 and the appliances run whatever Samsung shipped,
and it is partly self-limiting anyway, since add_observer drops a prior
observer for the same URI from the same addr/port and the library uses a fixed
source port.

If skipping them is deliberate, a line in the README beside the two-phase
example would cover it, so nobody reads that sequence as a drop-in for
close(). If not, draining the deregs before _lifecycle_cancel goes up would
keep both. Either way it can come later.

@QuiteYellow
QuiteYellow merged commit ea871fa into QuiteYellow:main Aug 29, 2026
8 checks passed
@Jason-Morcos

Copy link
Copy Markdown
Contributor Author

Agreed that close-notify and Observe cleanup are separate. I put the two-phase fix in #71.

quiesce_for_close() still wakes waiters and rejects new work immediately, but it now retains the active relation metadata. After the workers have joined, close() paces the exact deregistrations through a teardown-only send path, then flushes close-notify. That keeps the early shutdown hook quick without dropping the observer cleanup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants