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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sql: intermittent panic in the zstd response reader when SelectResults is closed

1 participant