Skip to content

Commit fd18df8

Browse files
authored
Merge pull request #117 from frstrtr/btc/v36-livelock-mirror-complete
btc(v36): complete livelock mirror — hold tracker lock across IO-thread share mutations
2 parents a504426 + e8abf28 commit fd18df8

1 file changed

Lines changed: 42 additions & 29 deletions

File tree

src/impl/btc/node.cpp

Lines changed: 42 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -393,34 +393,38 @@ void NodeImpl::processing_shares_phase2(HandleSharesData& data, NetService addr)
393393
// Non-blocking mutex: if think() holds the exclusive lock on the compute
394394
// thread, queue this batch for processing after think() releases. The IO
395395
// thread never blocks — keepalive timers and network I/O continue.
396-
{
397-
std::unique_lock lock(m_tracker_mutex, std::try_to_lock);
398-
if (!lock.owns_lock()) {
399-
// ── Backpressure (V36 livelock defense-in-depth) ──────────────
400-
// If think() is wedged/slow the deferred queue could grow without
401-
// bound and blow memory. Cap it: over MAX_PENDING_ADDS we DROP the
402-
// new batch (peers re-advertise their best share, so dropped shares
403-
// are re-requested later) and warn instead of growing unbounded.
404-
if (m_pending_adds.size() >= MAX_PENDING_ADDS) {
405-
LOG_WARNING << "[ASYNC-DEFER] BACKPRESSURE: pending_adds at cap ("
406-
<< m_pending_adds.size() << "/" << MAX_PENDING_ADDS
407-
<< "), dropping batch of " << data.m_items.size()
408-
<< " shares from " << addr.to_string()
409-
<< " — think() may be wedged";
410-
return;
411-
}
412-
LOG_INFO << "[ASYNC-DEFER] processing_shares_phase2: mutex busy, queuing "
413-
<< data.m_items.size() << " shares from " << addr.to_string()
414-
<< " (pending=" << m_pending_adds.size() + 1 << ")";
415-
m_pending_adds.push_back(PendingShareBatch{
416-
std::make_unique<HandleSharesData>(std::move(data)), addr});
396+
//
397+
// HOLD this lock across the entire mutation body below (mirrors LTC f445db8e).
398+
// try_to_lock keeps the IO thread non-blocking — busy => queue + return. Once
399+
// acquired we must NOT release until all m_tracker.chain mutations are done:
400+
// the prior code released here and ran the body lock-free, letting the
401+
// compute-thread clean_tracker() exclusive prune (drop_tails) free chain nodes
402+
// mid-mutation -> SIGSEGV (kr1z1s LTC/DGB). Released just before run_think().
403+
std::unique_lock lock(m_tracker_mutex, std::try_to_lock);
404+
if (!lock.owns_lock()) {
405+
// ── Backpressure (V36 livelock defense-in-depth) ──────────────
406+
// If think() is wedged/slow the deferred queue could grow without
407+
// bound and blow memory. Cap it: over MAX_PENDING_ADDS we DROP the
408+
// new batch (peers re-advertise their best share, so dropped shares
409+
// are re-requested later) and warn instead of growing unbounded.
410+
if (m_pending_adds.size() >= MAX_PENDING_ADDS) {
411+
LOG_WARNING << "[ASYNC-DEFER] BACKPRESSURE: pending_adds at cap ("
412+
<< m_pending_adds.size() << "/" << MAX_PENDING_ADDS
413+
<< "), dropping batch of " << data.m_items.size()
414+
<< " shares from " << addr.to_string()
415+
<< " — think() may be wedged";
417416
return;
418417
}
418+
LOG_INFO << "[ASYNC-DEFER] processing_shares_phase2: mutex busy, queuing "
419+
<< data.m_items.size() << " shares from " << addr.to_string()
420+
<< " (pending=" << m_pending_adds.size() + 1 << ")";
421+
m_pending_adds.push_back(PendingShareBatch{
422+
std::make_unique<HandleSharesData>(std::move(data)), addr});
423+
return;
419424
}
420-
// Lock released — proceed with normal processing.
421-
// No lock needed for the rest: we only enter here when think() is NOT
422-
// running (m_tracker_mutex was available), and ASIO single-thread
423-
// guarantees no other IO handler overlaps.
425+
// Lock acquired and HELD across the mutation body below; released just before
426+
// the async run_think() trigger so the compute thread can take the exclusive
427+
// lock. ASIO single-thread still guarantees no overlapping IO handler.
424428

425429
// Step 1: collect verified shares (skip any that failed verification, hash still null)
426430
std::vector<ShareType> valid_shares;
@@ -581,6 +585,11 @@ void NodeImpl::processing_shares_phase2(HandleSharesData& data, NetService addr)
581585
<< " chain=" << m_tracker.chain.size() << ")";
582586
}
583587

588+
// Release the tracker lock before triggering think(): run_think() posts the
589+
// think+prune job to the compute thread, which needs the exclusive lock. All
590+
// chain mutations above are complete at this point.
591+
lock.unlock();
592+
584593
// Trigger think() after every share batch (p2pool: set_best_share after handle_shares).
585594
// p2pool calls set_best_share() after EVERY batch with new_count > 0 — no size gate.
586595
// think() scores heads and updates best_share + desired set for download_shares.
@@ -805,14 +814,18 @@ void NodeImpl::notify_local_share(const uint256& share_hash)
805814
{
806815
// p2pool: set_best_share() → think() synchronously on the reactor thread.
807816
// Use think() for ALL best_share decisions, matching p2pool exactly.
808-
if (share_hash.IsNull() || !m_tracker.chain.contains(share_hash))
817+
if (share_hash.IsNull())
809818
return;
810819

811-
// Try inline verify — if think() holds the mutex, defer to next think() cycle.
812-
// The share is already in the chain; think() will verify + score it.
820+
// Both the chain.contains() read AND attempt_verify() run UNDER the tracker
821+
// lock (mirrors LTC f445db8e). A bare m_tracker.chain.contains() here (the
822+
// prior code) raced the compute-thread clean_tracker() exclusive prune freeing
823+
// chain nodes -> SIGSEGV (kr1z1s LTC/DGB). try_to_lock keeps the IO thread
824+
// non-blocking; if think()/clean holds the lock we skip the inline verify —
825+
// the share is already in-chain and run_think() below will score it next cycle.
813826
{
814827
std::unique_lock lock(m_tracker_mutex, std::try_to_lock);
815-
if (lock.owns_lock())
828+
if (lock.owns_lock() && m_tracker.chain.contains(share_hash))
816829
m_tracker.attempt_verify(share_hash);
817830
}
818831

0 commit comments

Comments
 (0)