Fix ReadBuffer is canceled exception in zip archive operations#100400
Fix ReadBuffer is canceled exception in zip archive operations#100400alexey-milovidov merged 1 commit intomasterfrom
Conversation
When a `ReadBuffer::nextImpl` throws, `ReadBuffer::next` calls `cancel()` and re-throws. The minizip library catches the exception via the C callback (`readFileFunc`) which stores it and returns 0. However, minizip may then call `readFileFunc` again before the stored exception is re-thrown via `rethrowStreamException`. At that point the underlying `ReadBuffer` is already canceled, and the `chassert(!isCanceled())` in `ReadBuffer::next` fires as a logical error. The fix adds early-return checks for `stored_exception` in all minizip callback functions (`readFileFunc`, `tellFunc`, `seekFunc`, `writeFileFunc`) to avoid touching the buffer after a prior exception has already canceled it. https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=100395&sha=97c66cfd486cfc33344b02f7bf69a66ce1664104&name_0=PR&name_1=Stress%20test%20%28amd_msan%29 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
|
Workflow [PR], commit [25984e2] Summary: ✅ AI ReviewSummaryThis PR addresses a real exception path in zip callbacks by short-circuiting callback bodies when PR Metadata
Suggested replacement: Findings
Tests
ClickHouse Rules
Final VerdictStatus: Minimum required actions:
|
clickhouse-gh
bot
added
the
pr-bugfix
![clickhouse-gh[bot]](https://avatars.githubusercontent.com/in/928874?s=60&v=4){kind=small}
LLVM Coverage Report
PR changed lines: PR changed-lines coverage: 85.71% (30/35, 0 noise lines excluded) |
robot-ch-test-poll1
added
the
pr-synced-to-cloud
3 participants
When a
ReadBuffer::nextImplthrows,ReadBuffer::nextcallscancel()and re-throws. The minizip library catches the exception via the C callback (readFileFunc) which stores it and returns 0. However, minizip may then callreadFileFuncagain before the stored exception is re-thrown viarethrowStreamException. At that point the underlyingReadBufferis already canceled, and thechassert(!isCanceled())inReadBuffer::nextfires as a logical error.The fix adds early-return checks for
stored_exceptionin all minizip callback functions (readFileFunc,tellFunc,seekFunc,writeFileFunc) to avoid touching the buffer after a prior exception has already canceled it.CI report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=100395&sha=97c66cfd486cfc33344b02f7bf69a66ce1664104&name_0=PR&name_1=Stress%20test%20%28amd_msan%29
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Fix
ReadBuffer is canceled. Can't read from it.exception in backup/restore operations using zip archives.Documentation entry for user-facing changes