proxy-io: Generalize ConnectStream / ServeStream for use in tests - #365
Open
ryanofsky wants to merge 4 commits into
Open
proxy-io: Generalize ConnectStream / ServeStream for use in tests#365ryanofsky wants to merge 4 commits into
ryanofsky wants to merge 4 commits into
Conversation
Add an accessor and a Connections type alias for the EventLoop's list of incoming connections, so future code can be simplified to locate a specific connection without directly accessing the private list or embedding a Connection object itself. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add a _Serve/ServeStream overload accepting the init object as a shared_ptr, so callers can transfer ownership instead of always passing a reference to an object they keep alive themselves. Existing reference-taking callers keep working through a thin overload that wraps the reference in a shared_ptr with an empty deleter. Also return the constructed ProxyServer along with an iterator to its Connection in loop.m_incoming_connections, so callers can look up or erase the connection later without embedding a Connection object themselves. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…Stream Give ServeStream and ConnectStream a destroy_connection parameter, defaulting to true, so callers can opt out of automatic connection teardown and manage the Connection's lifetime themselves instead. ServeStream gates the internal disconnect handler's list erase on the parameter; ConnectStream just forwards it to the existing ProxyClientBase parameter of the same name. This lets callers that need to keep a connection alive past a disconnect notification (e.g. to let in-flight server calls finish) use these helpers instead of constructing a Connection manually. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Replace TestSetup's manual Connection construction with ServeStream/ConnectStream, following the same pattern already used in Bitcoin Core's own IPC test and fuzz code. This drops server_on_disconnect entirely: ServeStream's destroy_connection parameter now controls whether a remote disconnect erases the server Connection, so the only test that needed to suppress that (the mp#348 getResults race test) just constructs TestSetup with server_owns_connection=false instead of overriding a callback afterward. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline and AI policy for information on the review process. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
This was referenced Sep 12, 2026
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
2 participants
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.
Generalize
ConnectStreamandServeStreamfunctions to simplify test setup in libmultiprocess and bitcoin core code (in followup ryanofsky/bitcoin@990f259). Also add anincomingConnectionsaccessor so Bitcoin Core code does not need to directly access the incoming connections list to disconnect clients.These changes help with #336 by getting rid of external code that would need to change with its changes to the
Connectionrepresentation.