Skip to content

Add Client.close/1 that sends Refresh(lifetime=0). Release 0.2.2#10

Open
sgfn wants to merge 3 commits intomasterfrom
fix/437-alloc-mismatch
Open

Add Client.close/1 that sends Refresh(lifetime=0). Release 0.2.2#10
sgfn wants to merge 3 commits intomasterfrom
fix/437-alloc-mismatch

Conversation

@sgfn
Copy link
Copy Markdown
Member

@sgfn sgfn commented May 5, 2026

As reported by @joaothallis, we're not tearing down existing allocations like we ought to. This PR extends the API so that ex_ice may cover this case and get rid of 437 Allocation Mismatch responses from the server.

joaothallis and others added 3 commits April 23, 2026 00:09
RFC 5766 §7.2 says a client gracefully releases its allocation by sending
a Refresh request with LIFETIME=0. Without it the server keeps the 5-tuple
bound until its own TTL expires (RFC default 600s), and a later Allocate
from the same source port earns 437 Allocation Mismatch (§6.2). This
deterministically reproduces against Cloudflare TURN when UDP source ports
are cycled rapidly.

Client.close/1 is a fire-and-forget emitter: the caller must ship the
returned datagram on the same socket the allocation was established on
before the socket is closed. The client transitions to :error on return,
so any late response is ignored.

Matches the teardown semantics in libwebrtc (TurnPort::Release), Pion
(UDPConn.Close → refreshAllocation(0, dontWait=true)), and aiortc
(TurnClientMixin.delete).
- Reuse refresh_request/5 (extended with optional Lifetime) from
  close/1 instead of inlining a near-identical Refresh construction.
- Pin client.ref in cancel_timers/1 mailbox flushes so notifications
  destined for another ExTURN.Client in the same owning process
  are not consumed by mistake.
- Trim Cloudflare-specific framing from the close/1 doc; the
  RFC-level rationale (5-tuple TTL, 437 on source-port reuse)
  stands on its own.
- Add a :alloc-state test for close/1 covering the post-401,
  pre-success transition the catch-all clause handles.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sgfn sgfn requested a review from Karolk99 May 5, 2026 11:09
@codecov
Copy link
Copy Markdown

codecov Bot commented May 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.32%. Comparing base (2e8da56) to head (4f7e315).

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #10      +/-   ##
==========================================
+ Coverage   72.16%   74.32%   +2.16%     
==========================================
  Files           8        8              
  Lines         273      296      +23     
==========================================
+ Hits          197      220      +23     
  Misses         76       76              
Files with missing lines Coverage Δ
lib/ex_turn/client.ex 74.01% <100.00%> (+2.58%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e8da56...4f7e315. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sgfn sgfn changed the title Fix/437 alloc mismatch Add Client.close/1 that sends Refresh(lifetime=0). Release 0.2.2 May 5, 2026
Copy link
Copy Markdown

@joaothallis joaothallis left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for fixing it!

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.

3 participants