Fix race condition causing 'ReadBuffer is canceled' exception#98955
Merged
alexey-milovidov merged 1 commit intomasterfrom Mar 7, 2026
Merged
Fix race condition causing 'ReadBuffer is canceled' exception#98955alexey-milovidov merged 1 commit intomasterfrom
alexey-milovidov merged 1 commit intomasterfrom
Conversation
…ter function callbacks When `receivePacketsExpectCancel` encounters an error reading from the client TCP connection, it throws an exception while holding `callback_mutex`. During stack unwinding, the mutex is released, and a pipeline worker thread waiting on the mutex can acquire it and attempt to read from the now-canceled `ReadBuffer` via `ClusterFunctionReadTaskCallback` (or similar callbacks). The worker thread passes `checkIfQueryCanceled` because `stop_query` hasn't been set yet (the main thread hasn't reached `executor.cancel()` yet). The worker then hits `chassert(!isCanceled())` in `ReadBuffer::next`. Fix: set `state.stop_query = true` in the catch block of `receivePacketsExpectCancel` before re-throwing, so that callbacks waiting on the mutex will see the cancellation via `checkIfQueryCanceled` immediately after acquiring it. Closes #98866 https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=98895&sha=d7e87bb2b7ecb3490ad63a12113fbc92fc8e488e&name_0=PR&name_1=BuzzHouse%20%28amd_msan%29 Co-Authored-By: Claude Opus 4.6 <[email protected]>
Contributor
clickhouse-gh
bot
added
the
pr-bugfix
Algunenano
added a commit
to Algunenano/ClickHouse
that referenced
this pull request
Mar 11, 2026
Followup to ClickHouse#98955. The previous fix only covered `receivePacketsExpectCancel`, but the same race exists between any two callbacks that read from the TCP `ReadBuffer` `*in`. When one callback's read fails (canceling the buffer), the exception unwinds and releases `callback_mutex`. Another callback waiting on the mutex can then acquire it and attempt to read from the now-canceled buffer before `checkIfQueryCanceled` sees the cancellation, triggering: "Logical error: 'ReadBuffer is canceled. Can't read from it.'" Fix: wrap the read sections of all four callbacks (`ExternalTablesInitializer`, `InputBlocksReaderCallback`, `ClusterFunctionReadTaskCallback`, `MergeTreeReadTaskCallback`) in try/catch that sets `state.stop_query = true` before rethrowing, matching the existing pattern in `receivePacketsExpectCancel`. Closes ClickHouse#99258 Co-Authored-By: Claude Opus 4.6 <[email protected]>
1 task
1 task
1 task
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.
Summary
TCPHandlerwherereceivePacketsExpectCancelcould cancel the TCPReadBufferand then releasecallback_mutexduring exception stack unwinding, allowing a pipeline worker thread to acquire the mutex and attempt to read from the canceled buffer before the executor is canceledcheckIfQueryCanceledbecausestop_queryhasn't been set yet, then hitschassert(!isCanceled())inReadBuffer::nextstate.stop_query = trueinreceivePacketsExpectCancel's catch block before re-throwing, so callbacks see the cancellation immediatelyCI report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=98895&sha=d7e87bb2b7ecb3490ad63a12113fbc92fc8e488e&name_0=PR&name_1=BuzzHouse%20%28amd_msan%29
Closes #98866
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Fix a race condition that could cause a "ReadBuffer is canceled" exception in queries using
urlClusteror similar cluster table functions.Documentation entry for user-facing changes
🤖 Generated with Claude Code
Note
Medium Risk
Touches TCP query-cancellation/error paths and changes how cancellation is signaled across threads, which could affect query termination behavior under network failures.
Overview
Fixes a race in
TCPHandler::receivePacketsExpectCancel()by wrapping cancel-packet polling/reading in atry/catchand settingstate.stop_query = truebefore rethrowing on any read/parsing error or disconnect.This ensures other callback-driven worker threads see cancellation via
checkIfQueryCanceled()immediately after the connection breaks, avoiding reads from a canceledReadBufferthat previously could trigger "ReadBuffer is canceled" exceptions.Written by Cursor Bugbot for commit e858e33. This will update automatically on new commits. Configure here.