Skip to content

Conversation

@hsanjuan
Copy link
Contributor

After boxo v0.33.1, this is a recommended step to fix http retrieval bugs.

Having a single ConnectEventManager prevents misdirected operations in the network.Router to change the Connectedness state in a way that the counterpart (httpnet or bsnet) can later correct.

After boxo v0.33.1, this is a recommended step to fix http retrieval bugs.

Having a single ConnectEventManager prevents misdirected operations in the
network.Router to change the Connectedness state in a way that the counterpart
(httpnet or bsnet) can later correct.
@hsanjuan hsanjuan self-assigned this Jul 31, 2025
@hsanjuan hsanjuan requested a review from a team as a code owner July 31, 2025 14:16
Copy link
Contributor

@gammazero gammazero left a comment

Choose a reason for hiding this comment

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

Was this something that could have lead to lost connections, if connected in one but not the other?

@hsanjuan
Copy link
Contributor Author

hsanjuan commented Aug 4, 2025

The theory is that httpnet.Connect/Disconnect races caused some Connects to be redirected to bsnet, which in turn generated and notified Disconnects that httpnet did not know about (so it did not reconnect).

It may well be that with all the other fixes this is not strictly necessary anymore, but it is a door that is better closed.

@hsanjuan
Copy link
Contributor Author

hsanjuan commented Aug 4, 2025

(I will merge myself once I add a changelog entry)

@hsanjuan hsanjuan added the skip/changelog This change does NOT require a changelog entry label Aug 6, 2025
@hsanjuan
Copy link
Contributor Author

hsanjuan commented Aug 6, 2025

After checking, it seems to be the changelog is high level enough that this boxo-fix does not need a special mention. The impact for Kubo was probably very minimal given how hard it was to trigger the race. So I'll leave things as they are.

@hsanjuan hsanjuan merged commit 58ad11b into master Aug 6, 2025
16 of 17 checks passed
@hsanjuan hsanjuan deleted the reuse-connevtmanager branch August 6, 2025 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip/changelog This change does NOT require a changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants