Fix ReadBuffer is canceled exception in zip archive operations

alexey-milovidov · claude · alexey-milovidov · commit 25984e22bc7c · 2026-03-23T00:51:09.000+01:00
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) <noreply@anthropic.com>
diff --git a/src/IO/Archives/ZipArchiveReader.cpp b/src/IO/Archives/ZipArchiveReader.cpp
@@ -130,6 +130,8 @@ namespace
         static unsigned long readFileFunc(void *, void * stream, void * buf, unsigned long size) // NOLINT(google-runtime-int)
         {
             auto & strm = get(stream);
+            if (strm.stored_exception)
+                return 0;
             try
             {
                 if (strm.at_end)
@@ -146,6 +148,8 @@ namespace
         static ZPOS64_T tellFunc(void *, void * stream)
         {
             auto & strm = get(stream);
+            if (strm.stored_exception)
+                return static_cast<ZPOS64_T>(-1);
             try
             {
                 if (strm.at_end)
@@ -162,6 +166,8 @@ namespace
         static long seekFunc(void *, void * stream, ZPOS64_T offset, int origin) // NOLINT(google-runtime-int)
         {
             auto & strm = get(stream);
+            if (strm.stored_exception)
+                return -1;
             try
             {
                 if (origin == SEEK_END)
diff --git a/src/IO/Archives/ZipArchiveWriter.cpp b/src/IO/Archives/ZipArchiveWriter.cpp
@@ -220,6 +220,8 @@ class ZipArchiveWriter::StreamInfo
     static unsigned long writeFileFunc(void * opaque, void *, const void * buf, unsigned long size) // NOLINT(google-runtime-int)
     {
         auto * stream_info = reinterpret_cast<StreamInfo *>(opaque);
+        if (stream_info->stored_exception)
+            return 0;
         try
         {
             stream_info->write_buffer->write(reinterpret_cast<const char *>(buf), size);
@@ -241,6 +243,8 @@ class ZipArchiveWriter::StreamInfo
     static ZPOS64_T tellFunc(void * opaque, void *)
     {
         auto * stream_info = reinterpret_cast<StreamInfo *>(opaque);
+        if (stream_info->stored_exception)
+            return static_cast<ZPOS64_T>(-1);
         try
         {
             auto pos = stream_info->write_buffer->count() - stream_info->start_offset;

Original file line number	Diff line number	Diff line change
`@@ -130,6 +130,8 @@ namespace`
`130`	`130`	`static unsigned long readFileFunc(void , void stream, void * buf, unsigned long size) // NOLINT(google-runtime-int)`
`131`	`131`	`{`
`132`	`132`	`auto & strm = get(stream);`
	`133`	`+ if (strm.stored_exception)`
	`134`	`+ return 0;`
`133`	`135`	`try`
`134`	`136`	`{`
`135`	`137`	`if (strm.at_end)`
`@@ -146,6 +148,8 @@ namespace`
`146`	`148`	`static ZPOS64_T tellFunc(void , void stream)`
`147`	`149`	`{`
`148`	`150`	`auto & strm = get(stream);`
	`151`	`+ if (strm.stored_exception)`
	`152`	`+ return static_cast<ZPOS64_T>(-1);`
`149`	`153`	`try`
`150`	`154`	`{`
`151`	`155`	`if (strm.at_end)`
`@@ -162,6 +166,8 @@ namespace`
`162`	`166`	`static long seekFunc(void , void stream, ZPOS64_T offset, int origin) // NOLINT(google-runtime-int)`
`163`	`167`	`{`
`164`	`168`	`auto & strm = get(stream);`
	`169`	`+ if (strm.stored_exception)`
	`170`	`+ return -1;`
`165`	`171`	`try`
`166`	`172`	`{`
`167`	`173`	`if (origin == SEEK_END)`
Original file line number	Diff line number	Diff line change
`@@ -220,6 +220,8 @@ class ZipArchiveWriter::StreamInfo`
`220`	`220`	`static unsigned long writeFileFunc(void * opaque, void , const void buf, unsigned long size) // NOLINT(google-runtime-int)`
`221`	`221`	`{`
`222`	`222`	`auto * stream_info = reinterpret_cast<StreamInfo *>(opaque);`
	`223`	`+ if (stream_info->stored_exception)`
	`224`	`+ return 0;`
`223`	`225`	`try`
`224`	`226`	`{`
`225`	`227`	`stream_info->write_buffer->write(reinterpret_cast<const char *>(buf), size);`
`@@ -241,6 +243,8 @@ class ZipArchiveWriter::StreamInfo`
`241`	`243`	`static ZPOS64_T tellFunc(void * opaque, void *)`
`242`	`244`	`{`
`243`	`245`	`auto * stream_info = reinterpret_cast<StreamInfo *>(opaque);`
	`246`	`+ if (stream_info->stored_exception)`
	`247`	`+ return static_cast<ZPOS64_T>(-1);`
`244`	`248`	`try`
`245`	`249`	`{`
`246`	`250`	`auto pos = stream_info->write_buffer->count() - stream_info->start_offset;`