Skip to content

fix: prevent deadlock in attestation signature aggregation#574

Merged
ch4r10t33r merged 1 commit intomainfrom
fix/attestation-deadlock
Feb 13, 2026
Merged

fix: prevent deadlock in attestation signature aggregation#574
ch4r10t33r merged 1 commit intomainfrom
fix/attestation-deadlock

Conversation

@ch4r10t33r
Copy link
Copy Markdown
Contributor

Summary

Fixes a potential deadlock in attestation signature aggregation by reducing the scope of the signatures_mutex lock.

Problem

The previous implementation held signatures_mutex across onBlock and updateHead calls, which could cause a deadlock:

  • Thread A (this): signatures_mutexforkChoice.mutex
  • Thread B (gossip): forkChoice.mutexsignatures_mutex

Solution

Lock signatures_mutex only for the duration of computeAggregatedSignatures, then immediately unlock before calling onBlock/updateHead.

Changes

  • Moved signatures_mutex.unlock() to immediately after computeAggregatedSignatures
  • Removed defer to make lock scope explicit
  • Added detailed comment explaining the deadlock scenario

Test Plan

  • Build succeeds
  • Tested under concurrent attestation load

@ch4r10t33r ch4r10t33r marked this pull request as ready for review February 13, 2026 14:38
@ch4r10t33r ch4r10t33r merged commit 2e27de9 into main Feb 13, 2026
12 checks passed
@ch4r10t33r ch4r10t33r deleted the fix/attestation-deadlock branch February 13, 2026 15:22
Comment on lines 339 to +348
@@ -343,6 +345,7 @@ pub const BeamChain = struct {
&self.forkChoice.aggregated_payloads,
);
_ = building_timer.observe();
self.forkChoice.signatures_mutex.unlock();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It's okay now that we have merged it but I find scoping it better design

{
        self.forkChoice.signatures_mutex.lock();
        defer self.forkChoice.signatures_mutex.unlock();
        const building_timer = zeam_metrics.lean_pq_sig_attestation_signatures_building_time_seconds.start();
        try aggregation.computeAggregatedSignatures(
            attestations,
            &self.forkChoice.aggregated_payloads,
        );
        _ = building_timer.observe();
}

I think the defer would be called as soon as the code section ends

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.

3 participants