stinger1206 opened a new issue, #67719:
URL: https://github.com/apache/doris/issues/67719
## Version
4.1.3 (branch-4.1; local build based on Doris commit 13a108d04b)
## What's Wrong?
Long-running BE nodes crash intermittently with SIGSEGV under concurrent
query/compaction load. Observed innermost frames:
- `SegmentFooterPB::ByteSizeLong` ← `Segment::get_metadata_size` (reader
side)
- `~SegmentFooterPB` ← `LRUCache::prune_if` (release side)
plus crashes at unrelated, seemingly random sites. Every observed crash is
healed by a BE restart without repair, and the implicated segment files remain
byte-identical. This argues against persistent on-disk corruption and is
consistent with process-memory corruption. Full `be.out` stacks from the
incident can be attached in a comment if needed.
Root cause is a data race on `std::weak_ptr<SegmentFooterPB>
Segment::_footer_pb` (`be/src/storage/segment/segment.h`):
- **Writer** — `Segment::_get_segment_footer()`
(`be/src/storage/segment/segment.cpp`) rebinds the weak_ptr (`_footer_pb =
footer_pb_shared`) from query/compaction threads whenever the previously cached
footer has expired (its `StoragePageCache` entry pruned/erased). It also reads
the weak_ptr via `_footer_pb.lock()`.
- **Reader** — `Segment::get_metadata_size()` calls `_footer_pb.lock()` from
the Daemon `memory_maintenance_thread`
(`MemoryProfile::refresh_memory_overview_profile` →
`MetadataAdder<Segment>::get_all_segments_estimate_size()`), with no
synchronization against the writers.
Concurrent assignment to `_footer_pb` and `lock()` on the same non-atomic
`std::weak_ptr` object is a data race and therefore undefined behavior. The
exact failure mechanism is implementation-dependent; the observed crashes are
consistent with process-memory corruption caused by this undefined behavior.
The same unsynchronized pattern is present in both current `master` and
`branch-4.1`.
## What You Expected?
Concurrent footer-cache expiry (query/compaction) and periodic
memory-maintenance size estimation should be safe; BE should not crash.
## How to Reproduce?
Hard to reproduce via SQL alone: it requires segment footer cache entries
expiring while the memory maintenance thread's refresh runs (a matter of timing
under sustained load). It reproduces intermittently in production on
long-running BEs under mixed query + compaction traffic.
The exact C++ pattern reproduces reliably under ThreadSanitizer with a
standalone, dependency-free program that mimics the two access paths
(`_get_segment_footer` rebind vs `get_metadata_size` lock):
```cpp
// repro_weakptr_race.cpp — micro-repro of the Segment::_footer_pb pattern
// clang++ -std=c++17 -O1 -g -fsanitize=thread repro_weakptr_race.cpp -o
repro_tsan && ./repro_tsan
#include <chrono>
#include <cstdio>
#include <memory>
#include <thread>
#include <vector>
struct FooterPB {
char payload[64];
size_t ByteSizeLong() const { return sizeof(FooterPB); }
};
std::weak_ptr<FooterPB> g_footer_pb; // == Segment::_footer_pb
// "Segment::_get_segment_footer" writer (query / compaction thread):
// parse footer, insert into footer cache, rebind the weak_ptr.
void writer_loop() {
uint64_t n = 0;
while (true) {
auto footer = std::make_shared<FooterPB>();
footer->payload[0] = static_cast<char>(n++ & 0xff);
g_footer_pb = footer; // unlocked rebind
std::this_thread::sleep_for(std::chrono::microseconds(50));
}
}
// "Segment::get_metadata_size" reader (Daemon memory_maintenance_thread):
void reader_loop() {
while (true) {
auto footer = g_footer_pb.lock(); // unlocked lock()
if (footer) {
volatile size_t sz = footer->ByteSizeLong();
(void)sz;
}
std::this_thread::sleep_for(std::chrono::microseconds(50));
}
}
int main() {
std::vector<std::thread> ts;
for (int i = 0; i < 4; i++) ts.emplace_back(writer_loop);
for (int i = 0; i < 4; i++) ts.emplace_back(reader_loop);
for (auto& t : ts) t.join(); // never reached; Ctrl-C after the first
TSAN report
return 0;
}
```
In our test TSAN reports the race within seconds, with one access in
`std::weak_ptr::operator=` and the other in `std::weak_ptr::lock`.
## Anything Else?
Existing issue #64826 reports an overlapping SegmentFooterPB crash symptom
during page-cache capacity adjustment, but does not report or identify the
`Segment::_footer_pb` data race described here.
Fix: serialize every `_footer_pb` access with a dedicated `mutable
std::mutex _footer_pb_lock` in `Segment` — read in `get_metadata_size()`, read
+ rebind in `_get_segment_footer()`. The mutex is `mutable` because
`get_metadata_size()` is `const`; the `shared_ptr` is copied under the lock and
`ByteSizeLong()` runs outside it, so the critical sections are a few
instructions. No behavior change. Validated on a production canary BE (crash
family absent after deploy; previously recurring).
- [x] I had searched in the issues and found no similar issues.
- [x] Yes I am willing to submit a PR!
- [x] I agree to follow this project's [Code of
Conduct](https://www.apache.org/foundation/policies/conduct).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
