[Bug] BE SIGSEGV caused by data race on Segment::_footer_pb

[stinger1206]

Version

4.1.3 (branch-4.1; local build based on Doris commit 13a108d)

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):

// 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).

• I had searched in the issues and found no similar issues.
• Yes I am willing to submit a PR!
• I agree to follow this project's Code of Conduct.

[Doris-Breakwater]

Initial triage

Verdict: this report identifies a credible, code-confirmed concurrency-safety defect, but the attribution of the observed crash family to the stated daemon-thread access path is not yet confirmed. I recommend treating the synchronization fix as valid while requesting the incident evidence below before declaring the production root cause proven.

Verified facts

• The issue is currently open with no labels, assignee, milestone, or linked PR.
• At the reported commit 13a108d04b, Segment::_footer_pb is a plain std::weak_ptr<SegmentFooterPB>. Segment::get_metadata_size() and Segment::_get_segment_footer() call lock() on that same member, while _get_segment_footer() also assigns to it, without a mutex protecting the member.
• The same pattern is still present in upstream master at 2e89f3a81cbf0effe06491a383109ab36ec72923 and branch-4.1 at 73fea8a58956d020231bdc053029107e8451f37c (the branch tips checked during this triage).
• Concurrent operations on the same non-atomic std::weak_ptr object require synchronization. Therefore, if two threads reach these accesses on the same Segment, the code has undefined behavior.
• Such same-object concurrency is mechanically possible: SegmentLoader caches and returns shared Segment instances, and not all post-open _get_segment_footer() callers share one mutex. ColumnReaderCache serializes its own footer-callback calls with _cache_mutex, but Segment::traverse_column_meta_pbs() calls _get_segment_footer() directly. Thus the standalone TSAN example demonstrates the relevant language-level defect, and the Doris code contains reachable access paths that should not rely on an undocumented single-thread invariant.

Important discrepancy / evidence gap

The daemon call chain stated in the issue does not exist in the referenced upstream source. At 13a108d04b (and at both branch tips above), MemoryProfile::refresh_memory_overview_profile() calls MetadataAdder<Segment>::get_all_segments_estimate_size(), which only reads g_segment_estimate_mem_bytes; it does not enumerate Segment objects or call Segment::get_metadata_size().

Consequently, the following should be separated:

• Confirmed: _footer_pb lacks the synchronization required for concurrent access.
• Plausible but not yet proven: this race caused the reported ByteSizeLong, destructor, and unrelated SIGSEGV crashes.
• Currently contradicted by upstream source: memory_maintenance_thread -> refresh_memory_overview_profile -> get_all_segments_estimate_size -> Segment::get_metadata_size at the stated commit.

The overlapping destructor symptom in #64826 is worth cross-referencing, but it is not enough to establish that the two issues have the same root cause.

Information requested

1. Please attach at least one complete, symbolized be.out/core backtrace for the Segment::get_metadata_size() crash and one for the SegmentFooterPB destructor crash, including all frames above the shown functions, thread names, timestamps, and binary build ID.
2. Because this is a local build, please provide the source diff from 13a108d04b for at least metadata_adder.h, memory_profile.cpp, segment.h, and segment.cpp (or confirm that these files are byte-identical to upstream). This is necessary to reconcile the daemon call-chain discrepancy.
3. If available, attach a TSAN report from a Doris test/build that names the actual Segment::_footer_pb accesses. The standalone reproducer proves the C++ pattern but not the production execution pair.
4. For the canary result, please include observation duration/load and the pre-fix recurrence baseline; absence of crashes is supportive but not independently conclusive for an intermittent failure.

Recommended next step

The proposed dedicated mutable std::mutex is an appropriate minimal fix for C++17. The PR should protect every access to the _footer_pb member: both lock() sites and the rebind. Copy the resulting shared_ptr while holding the mutex, then perform ByteSizeLong(), footer parsing, page-cache lookup/insert, and I/O outside the critical section. This removes the undefined behavior without holding a segment-local lock across expensive work; duplicate cache-miss work remains possible, as it is today.

Please add or run a concurrency-focused test under TSAN if the project test setup supports it, plus the relevant BE unit tests. A normal non-TSAN stress test alone may not reliably detect this race. Once a PR is opened, link it here and target both master and branch-4.1 according to the project backport policy.

Breakwater-GitHub-Analysis-Slot: slot_b3ebbbe1cd9e