Fix table lifetime in case of parallel DROP TABLE and INSERT #32572
Conversation
This file contains 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
Stress tests founds [1]:
==527==WARNING: MemorySanitizer: use-of-uninitialized-value
0 0x37078ffd in unsigned long std::__1::__cxx_atomic_fetch_add<unsigned long>(std::__1::__cxx_atomic_base_impl<unsigned long>*, unsigned long, std::__1::memory_order) obj-x86_64-linux-gnu/../contrib/libcxx/include/atomic:1050:12
1 0x37078ffd in std::__1::__atomic_base<unsigned long, true>::fetch_add(unsigned long, std::__1::memory_order) obj-x86_64-linux-gnu/../contrib/libcxx/include/atomic:1719:17
2 0x37078ffd in std::__1::__atomic_base<unsigned long, true>::operator++() obj-x86_64-linux-gnu/../contrib/libcxx/include/atomic:1756:57
3 0x37078ffd in SimpleIncrement::get() obj-x86_64-linux-gnu/../src/Common/SimpleIncrement.h:20:16
4 0x37078ffd in DB::MergeTreeDataWriter::writeTempPart(DB::BlockWithPartition&, std::__1::shared_ptr<DB::StorageInMemoryMetadata const> const&, std::__1::shared_ptr<DB::Context const>) obj-x86_64-linux-gnu/../src/Storages/MergeTree/MergeTreeDataWriter.cpp:276:46
5 0x373c446c in DB::MergeTreeSink::consume(DB::Chunk) obj-x86_64-linux-gnu/../src/Storages/MergeTree/MergeTreeSink.cpp:27:65
Uninitialized value was created by a heap deallocation
6 0x32d481e8 in DB::DatabaseCatalog::TableMarkedAsDropped::~TableMarkedAsDropped() obj-x86_64-linux-gnu/../src/Interpreters/DatabaseCatalog.h:248:12
7 0x32d3c134 in DB::DatabaseCatalog::dropTableDataTask() obj-x86_64-linux-gnu/../src/Interpreters/DatabaseCatalog.cpp:908:1
[1]: https://s3.amazonaws.com/clickhouse-test-reports/32534/fa6090f588dbf4cbb5f28bd2210847b070bb8218/stress_test__memory__actions_.html
The query was CREATE MATERIALIZED VIEW ... POPULATE AS SELECT ... from
00040_aggregating_materialized_view test.
robot-clickhouse
added
the
pr-bugfix
|
@azat Something is wrong with Kerberized Kafka test. |
alexey-milovidov
approved these changes
Dec 12, 2021
tavplubix
approved these changes
Dec 12, 2021
This was referenced Dec 12, 2021
Merged
Merged
Closed
Closed
Closed
alexey-milovidov
added a commit
that referenced
this pull request
Dec 14, 2021
Backport #32572 to 21.12: Fix table lifetime in case of parallel DROP TABLE and INSERT
tavplubix
added a commit
that referenced
this pull request
Dec 16, 2021
Backport #32572 to 21.11: Fix table lifetime in case of parallel DROP TABLE and INSERT
azat
added a commit
to azat/ClickHouse
that referenced
this pull request
Jan 4, 2022
ASan founds [1]:
==553==
ERROR: AddressSanitizer: heap-use-after-free on address 0x61e004694080 at pc 0x000029150af2 bp 0x7f70b3f8ef10 sp 0x7f70b3f8ef08
READ of size 8 at 0x61e004694080 thread T477 (QueryPipelineEx)
0 0x29150af1 in DB::MergeTreeDataWriter::writeTempPart() >
1 0x293b8e43 in DB::MergeTreeSink::consume(DB::Chunk) obj-x86_64-linux-gnu/../src/Storages/MergeTree/MergeTreeSink.cpp:27:65
2 0x29dac73b in DB::SinkToStorage::onConsume(DB::Chunk) obj-x86_64-linux-gnu/../src/Processors/Sinks/SinkToStorage.cpp:18:5
3 0x29c72dd2 in DB::ExceptionKeepingTransform::work()::$_1::operator()() const obj-x86_64-linux-gnu/../src/Processors/Transforms/ExceptionKeepingTransform.cpp:151:51
0x61e004694080 is located 2048 bytes inside of 2480-byte region [0x61e004693880,0x61e004694230)
freed by thread T199 (BgSchPool) here:
...
4 0x26220f20 in DB::DatabaseCatalog::TableMarkedAsDropped::~TableMarkedAsDropped() obj-x86_64-linux-gnu/../src/Interpreters/DatabaseCatalog.h:248:12
5 0x26220f20 in DB::DatabaseCatalog::dropTableDataTask() obj-x86_64-linux-gnu/../src/Interpreters/DatabaseCatalog.cpp:908:1
[1]: https://s3.amazonaws.com/clickhouse-test-reports/33201/4f04d6af61eabf4899eb8188150dc862aaab80fc/stress_test__address__actions_.html
There was a fix in ClickHouse#32572, but it was not complete (yes it reduced the
race window a lot, but not completely), since the inner table still can
go away after the INSERT chain was built, to fix this obtain the
reference earlier.
Follow-up for: ClickHouse#32572 (cc @tavplubix)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fix table lifetime (i.e. possible use-after-free) in case of parallel DROP TABLE and INSERT
Detailed description / Documentation draft:
Stress tests founds 1:
The query was CREATE MATERIALIZED VIEW ... POPULATE AS SELECT ... from
00040_aggregating_materialized_view test.