Skip to content

fix: avoid double-closing S3 Select responses - #24

Merged
Vonng merged 1 commit into
mainfrom
codex/fix-sql-select-close
Aug 31, 2026
Merged

fix: avoid double-closing S3 Select responses#24
Vonng merged 1 commit into
mainfrom
codex/fix-sql-select-close

Conversation

@Vonng

@Vonng Vonng commented Aug 31, 2026

Copy link
Copy Markdown
Member

Fixes #23.

Root cause

minio-go SelectResults owns the HTTP response and closes it from its background event-stream reader on every terminal path. The caller then deferred a second SelectResults.Close. With gzhttp zstd decoding, the two close paths concurrently drained the same unsynchronized decoder and could return it to the pool twice.

Fix

  • wrap SelectResults at the S3Client.Select boundary with one safe ReadCloser contract;
  • give each Select request a child context;
  • on Close, cancel the request and drain the result pipe so the SDK background reader reaches its sole response-close path;
  • serialize Read and early Close, and make Close idempotent;
  • never invoke the SDK Close that owns the conflicting second response drain.

Verification

  • deterministic zstd event-stream regression, including a 256 KiB End-event tail;
  • real open zstd stream: read one record, close before End, assert request cancellation;
  • deterministic in-flight Read versus Close test;
  • focused race tests, 20 repetitions;
  • real SILO RELEASE.2026-08-06T00-00-00Z: 100/100 single-object SQL and 20/20 recursive multi-object SQL, zero panic and zero timeout;
  • full repository race tests and all verifiers;
  • silo-console v2.2.1 full Go suite with this mc revision;
  • current Silo cmd package compilation with this mc revision.

The branch changes only mc. No silo-pkg or silo-console release is required.

Fixes #23.

Signed-off-by: Feng Ruohang <rh@vonng.com>
@Vonng
Vonng merged commit ddac5d5 into main Aug 31, 2026
8 checks passed
@Vonng
Vonng deleted the codex/fix-sql-select-close branch August 31, 2026 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant