Skip to content

fix(database): abort fork switch when pop_block() makes no progress (write-lock deadlock) - #117

Merged
On1x merged 1 commit into
masterfrom
fix/fork-switch-pop-no-progress-deadlock
Jun 16, 2026
Merged

fix(database): abort fork switch when pop_block() makes no progress (write-lock deadlock)#117
On1x merged 1 commit into
masterfrom
fix/fork-switch-pop-no-progress-deadlock

Conversation

@chiliec

@chiliec chiliec commented Jun 16, 2026

Copy link
Copy Markdown
Member

A minority-fork node attempting to reorg onto the majority chain could wedge: push_block() holds the global chainbase write lock and never releases it (observed writer_held_ms climbing for hours), timing out every reader and blocking the validator's own block production.

Root cause: the three fork-switch pop loops in _push_block() only break when head_block_num() <= last_irreversible_block_num. pop_block() reverts head via undo(); if undo() has nothing to revert (committed/empty undo session) while head is still ABOVE LIB, head_block_id() never changes and head_block_num() never decreases to reach the LIB guard. The loop spins forever holding the write lock.

Fix: after each pop_block(), detect that head_block_id() did not change and abort the loop (main switch: remove the new head from fork_db and throw unlinkable_block_exception so the P2P layer recovers cleanly; recovery loops: break). This guarantees the loop terminates and the write lock is released, so a failed reorg degrades to 'stay on current fork + resync' instead of wedging the node.

…write-lock deadlock)

A minority-fork node attempting to reorg onto the majority chain could wedge:
push_block() holds the global chainbase write lock and never releases it
(observed writer_held_ms climbing for hours), timing out every reader and
blocking the validator's own block production.

Root cause: the three fork-switch pop loops in _push_block() only break when
head_block_num() <= last_irreversible_block_num. pop_block() reverts head via
undo(); if undo() has nothing to revert (committed/empty undo session) while
head is still ABOVE LIB, head_block_id() never changes and head_block_num()
never decreases to reach the LIB guard. The loop spins forever holding the
write lock.

Fix: after each pop_block(), detect that head_block_id() did not change and
abort the loop (main switch: remove the new head from fork_db and throw
unlinkable_block_exception so the P2P layer recovers cleanly; recovery loops:
break). This guarantees the loop terminates and the write lock is released, so
a failed reorg degrades to 'stay on current fork + resync' instead of wedging
the node.

@On1x On1x left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

все хорошо

@On1x
On1x merged commit f2c37ca into master Jun 16, 2026
1 check passed
@chiliec
chiliec deleted the fix/fork-switch-pop-no-progress-deadlock branch June 16, 2026 04:24
web3blind pushed a commit to web3blind/viz-cpp-node that referenced this pull request Jul 3, 2026
…rogress

VIZ-Blockchain#117 added a no-progress guard to the three fork-switch pop loops in
_push_block() but left the fourth pop loop — the P39 cascade-recovery
path — unguarded. It has the identical deadlock: when pop_block()'s
undo() is a no-op (committed/empty undo session while head is still
above LIB), head_block_num() never decreases, so neither
'> original_head' nor '> lib_num' can ever become false and the loop
spins forever holding the global chainbase write lock — the same wedge
(writer_held_ms climbing for hours) observed during the minority-fork
reorg incident.

Mirror VIZ-Blockchain#117's recovery-loop pattern: detect that head_block_id() did
not change after pop_block() and break. The existing _fork_db.reset()
+ start_block() right after the loop already restores fork_db to the
database head, so break leaves consistent state. This closes the last
unguarded no-progress pop loop in _push_block().
web3blind pushed a commit to web3blind/viz-cpp-node that referenced this pull request Jul 3, 2026
Observability backstop for the global chainbase write lock. A wedged push_block()
holding the write lock takes the whole node down (every reader + the validator's
own block production block on it; observed: writer_held_ms climbing for hours
during a minority-fork reorg).

A monitor thread watches the per-push_block hold time and, when a single
push_block holds the write lock past PUSH_BLOCK_STALL_WARN_SEC (60s) with no
operation applied in that window, emits a loud, throttled stderr diagnostic. It
also exposes push_block_lock_held_ms() for external healthchecks.

LOG-ONLY by design: it never calls std::_Exit and never mutates state, so a false
positive can only add a log line, never a node death. A force-exit watchdog was
deliberately rejected: on a deterministic heavy block (e.g. a future hardfork) it
would self-destruct the whole network at once -- far worse than the localized hang
it would guard against. Restart decisions stay with the operator / orchestrator,
driven by push_block_lock_held_ms().

Complements the pop-loop no-progress guards (VIZ-Blockchain#117/VIZ-Blockchain#118): those fix the known
infinite loops; this only makes any remaining/future write-lock stall visible.
chiliec added a commit that referenced this pull request Jul 13, 2026
A snapshot-synced node whose state is missing an object rejects every
canonical block that references it (out_of_range at apply_block). This
is a rejection livelock, distinct from the #117-#120 write-lock deadlock
monitors: push_block fails fast, so those monitors never fire. The
divergence sits at/below LIB, so fork-switch cannot recover — only
wipe + snapshot re-import does, and that was gated on empty state, so it
never self-triggered. The 2026-07-13 rpc.viz.cx outage needed a manual
docker compose --force-recreate after ~14h frozen.

Detection (dlt_p2p_node::check_wedge_watchdog, in periodic_task):
declare WEDGED when, sustained for WEDGE_CONFIRM_SEC (900s):
  behind > WEDGE_BEHIND_THRESHOLD (200) AND head flat AND gap-fill
  rejections still climbing. The triple-AND rules out a healthy syncing
node (head advancing) and a partition (behind+flat but no rejections).
A new monotonic _gap_rejected_total drives the "still rejecting" signal
because _gap_rejected_count oscillates (resets on blacklist). The pure
predicate is_wedged() is factored out for table-testing.

Action (gated behind auto-resync-on-wedge, default OFF for this release
so we observe the elog/marker in the wild first): elog, write a
force_resync marker next to shared_memory.bin, then std::_Exit(2)
(precedent: undo_all watchdog). With OFF, the watchdog only logs.

Recovery (chain plugin_startup): a force_resync marker wipes state so
head becomes 0, and the existing empty-state gate re-bootstraps from the
trusted snapshot peer, then deletes the marker — the exact path a manual
--force-recreate triggers today. The marker is removed only after the
wipe, so a crash mid-wipe re-triggers recovery; it composes with the
resize marker (wipe clears it too). The marker lives in the shared-memory
dir (exposed via chain::plugin::get_state_dir) so it persists exactly as
long as the state it condemns.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants