Skip to content

fix(go/adbc/driver/flightsql): drain bound-param stream in testserver DoPut#4497

Open
fornwall wants to merge 2 commits into
apache:mainfrom
fornwall:fix/testserver-drain
Open

fix(go/adbc/driver/flightsql): drain bound-param stream in testserver DoPut#4497
fornwall wants to merge 2 commits into
apache:mainfrom
fornwall:fix/testserver-drain

Conversation

@fornwall

@fornwall fornwall commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

The FlightSQL test server's DoPutPreparedStatementQuery handler returned without consuming the client's bound-parameter request stream. gRPC tears the stream down as soon as the handler returns, so it might happen that (observed on arm64 macOS CI) the client's in-flight parameter send races the teardown and surfaces as a spurious "EOF (Unknown; ExecuteQuery)" instead of the real response, flaking test_stateless_prepared_statement.

Drain the reader before returning so the handler consumes the full request stream, per gRPC's contract, removing the race.

fornwall added 2 commits July 10, 2026 00:45
It seems that AddressSanitizer (ASan) here hangs CI at:

- Native Libraries (Unix) / C/C++ (Conda/macos-latest)

Example run on latest main branch commit at time of writing:

- https://github.com/apache/arrow-adbc/actions/runs/28991039011/job/86030648737

Disable ASan on macos-latest for now to work around that.

Signed-off-by: Fredrik Fornwall <fredrik@fornwall.net>
… DoPut

The FlightSQL test server's DoPutPreparedStatementQuery handler returned
without consuming the client's bound-parameter request stream. gRPC tears
the stream down as soon as the handler returns, so on a slow/oversubscribed
runner (observed on arm64 macOS CI) the client's in-flight parameter send
races the teardown and surfaces as a spurious "EOF (Unknown; ExecuteQuery)"
instead of the real response, flaking test_stateless_prepared_statement.

Drain the reader before returning so the handler consumes the full request
stream, per gRPC's contract, removing the race.

Signed-off-by: Fredrik Fornwall <fredrik@fornwall.net>
@lidavidm

lidavidm commented Jul 9, 2026

Copy link
Copy Markdown
Member

Good find here, but the client should also be fixed to not just report an EOF, if your agent can figure that out...

@fornwall

fornwall commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

This includes the changes in #4495, as that is necessary to unblock CI and reach the test that this PR aims to fix. But that change can be reviewed and merged in isolation - this test failure surfaces after that fix, but is AFAIK separate.

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