Skip to content

Conversation

@aler9
Copy link
Contributor

@aler9 aler9 commented Dec 22, 2025

related issue: bluenviron/mediamtx#3756

When listening and accepting an incoming connection request, the response might be received by the peer with some delay due to latency. This causes the peer to send a second connection request, that is not detected as duplicate because the first connection request has already been removed from the map that is used to check for duplicates
(connReqs), so it is treated as a brand new connection request, breaking the first connection.

This patch fixes the issue by introducing another map (connByPeer) that is used to check whether a connection request is associated to an already-accepted connection.

@gemini-code-assist
Copy link

Important

Installation incomplete: to start using Gemini Code Assist, please ask the organization owner(s) to visit the Gemini Code Assist Admin Console and sign the Terms of Services.

@codecov-commenter
Copy link

codecov-commenter commented Dec 22, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 78.57143% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.35%. Comparing base (d421390) to head (d7fe54d).

Files with missing lines Patch % Lines
conn_request.go 71.42% 0 Missing and 2 partials ⚠️
connection.go 50.00% 1 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #123      +/-   ##
==========================================
- Coverage   56.38%   56.35%   -0.04%     
==========================================
  Files          22       22              
  Lines        4182     4181       -1     
==========================================
- Hits         2358     2356       -2     
  Misses       1510     1510              
- Partials      314      315       +1     
Flag Coverage Δ
unit-linux 56.35% <78.57%> (-0.04%) ⬇️

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.

When listening and accepting an incoming connection request, the
response might be received by the peer with some delay due to latency.
This causes the peer to send a second connection request, that is not
detected as duplicate because the first connection request has already
been removed from the map that is used to check for duplicates
(connReqs), so it is treated as a brand new connection request,
breaking the first connection.

This patch fixes the issue by replacing the map that is used to check
for duplicates (connReqs) with a map that keeps track of both
connection requests and accepted connections by their peer ID
(connByPeers), allowing to discard handshakes with an existing peer ID.
@aler9 aler9 force-pushed the patch-duplicate-conns branch from c99e7d2 to d7fe54d Compare December 22, 2025 19:37
@philipthomasme
Copy link

Very important fix. Thanks, @aler9
What's the merge schedule, @ioppermann ?

@ioppermann ioppermann self-assigned this Dec 30, 2025
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.

4 participants