Don't reset shared receivers from a UDP tunnel that was never started - #2543
Merged
Merged
Conversation
Contributor
Author
|
Confirmed once more on the current head 920226d, without this branch: with zero sanitizer reports, so the breakage is silent - nothing crashes, the tunnel object is alive and the destination is up, only the receiver is gone. |
Contributor
Author
|
I checked the neighbouring tunnels for the same shape. Only the UDP tunnels define a destructor that calls |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A configuration reload silently stops UDP tunnels from delivering anything. The tunnel keeps running, the destination stays, nothing is recreated - and every datagram that arrives is dropped.
Mechanism:
ClientContextbuilds the tunnel object first and only then discovers that the same forward already exists (libi2pd_client/ClientContext.cpp:911for a server forward,:740for a client one). The extra object is discarded, its destructor runsStop (), andStop ()callsResetReceiveron the datagram destination - which is shared with the live tunnel. From that moment the live tunnel has no receiver.The server log says exactly that:
Datagram: no receiver for port 0, 65 times after the reload.Reproduction, a scenario that checks what must still work after a reload - a page through the http proxy, a datagram through a udp tunnel pair, and a SAM answer:
A control run of the same scenario without reloads keeps all three at 1 in all three checks, so it is the reload and not elapsed time or the network. In the logs the client keeps sending (its sequence number grows, acks time out) while the server's session shows nothing received.
With this change, on the same scenario and the same bench:
and
no receiver for portnever appears in the log. Zero sanitizer reports in both runs.The fix keeps a tunnel from tearing down shared state it never set up:
Start ()marks the tunnel started, andStop ()returns early if it was not. The client tunnel gets the same guard - itsStop ()resets the receiver in the same way.makehas no new warnings,make -C testspasses,g++ -std=c++17 -fsyntax-onlyon the changed file passes.