[Bug] BE SIGSEGV caused by data race on Segment::_footer_pb
- Dominant language
- Java
- Stars
- 15.9k
- Forks
- 3.9k
- Avg merge
- 2d 23h
- Merged PRs (30d)
- 520
Description
## 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 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::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
#include
#include
#include
#include
struct FooterPB {
char payload[64];
size_t ByteSizeLong() const { return sizeof(FooterPB); }
};
std::weak_ptr 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();
footer->payload[0] = static_cast(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 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).
Contributor guide
Research direction
Read be/src/storage/segment/segment.h and segment.cpp, focusing on Segment::_get_segment_footer() and get_metadata_size(), then review the provided ThreadSanitizer reproducer. Done means all _footer_pb accesses are synchronized while footer size calculation remains safe, with no race reported for the concurrent cache-expiry and metadata-estimation paths.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- database
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 74/100