Skip to content

Fix handshake hang when Finished arrives with retransmitted state#808

Merged
JoTurk merged 1 commit intopion:masterfrom
662a34:fix/retransmit-finished-lost
Mar 5, 2026
Merged

Fix handshake hang when Finished arrives with retransmitted state#808
JoTurk merged 1 commit intopion:masterfrom
662a34:fix/retransmit-finished-lost

Conversation

@662a34
Copy link
Contributor

@662a34 662a34 commented Mar 1, 2026

Description

The retransmit flag is set per-datagram, not per-record: conn.go#L818-L847.

I ran into a production case where Finished sometimes shares a datagram with retransmitted records.
In a scenario like this, the handshaker skips parsing, cannot finish the handshake and hungs indefinitely.

This PR aims to retain the backoff logic for retransmits (#758), while allowing an edge case like this to be parsed. The included test reproduces the issue.

Reference issue

Follow up to #758 and subsequent fixes.

@codecov
Copy link

codecov bot commented Mar 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.51%. Comparing base (a25c8b8) to head (0d9b7c1).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #808      +/-   ##
==========================================
+ Coverage   82.43%   82.51%   +0.08%     
==========================================
  Files         120      120              
  Lines        6775     6773       -2     
==========================================
+ Hits         5585     5589       +4     
+ Misses        777      774       -3     
+ Partials      413      410       -3     
Flag Coverage Δ
go 82.51% <100.00%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

@662a34 662a34 force-pushed the fix/retransmit-finished-lost branch from 8a82f73 to 16dd15c Compare March 1, 2026 07:46
Copy link
Member

@JoTurk JoTurk left a comment

Choose a reason for hiding this comment

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

Thank you

The retransmit flag is set per-datagram, not per-record. When a new
Finished shares a datagram with retransmitted records, the handshaker
skips parsing and hangs.  Always parse; only suppress the backoff
timer reset for retransmits (pion#758).
@JoTurk JoTurk force-pushed the fix/retransmit-finished-lost branch from 16dd15c to 0d9b7c1 Compare March 5, 2026 15:46
@JoTurk JoTurk merged commit 4fcce60 into pion:master Mar 5, 2026
18 of 19 checks passed
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