Skip to content

fix(relay): avoid subscription lock inversion - #3413

Merged
tlongwell-block merged 4 commits into
mainfrom
wren/fix-subscription-lock-order
Jul 29, 2026
Merged

fix(relay): avoid subscription lock inversion#3413
tlongwell-block merged 4 commits into
mainfrom
wren/fix-subscription-lock-order

Conversation

@tlongwell-block

Copy link
Copy Markdown
Collaborator

Summary

  • drop the subs DashMap guard before mutating subscription indexes
  • snapshot fan-out candidate vectors so index guards are dropped before looking up subs
  • add concurrent fan-out/replacement regression coverage

Why

fan_out_scoped previously held an index guard while push_match acquired subs, while CLOSE and same-ID replacement held subs while removing from an index. The reverse ordering made an AB/BA deadlock reachable and could synchronously park all Tokio workers.

Validation

  • rustup run 1.95.0 cargo test -p buzz-relay — 769 library tests passed, 33 ignored; 11 binary tests passed; doc tests passed
  • push hooks with pinned Rust 1.95 — branch-skew, repository Rust suites, and desktop Tauri suite passed
  • git diff --check

Residual risk

Fan-out now clones bounded candidate vectors before matching. This adds allocation/copy cost proportional to the indexed candidate set, in exchange for eliminating nested DashMap guards. This fixes the concrete lock cycle but does not prove every observed production wedge had this cause.

Co-authored-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@buzz.block.builderlab.xyz>
Signed-off-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@buzz.block.builderlab.xyz>
@tlongwell-block
tlongwell-block requested a review from a team as a code owner July 28, 2026 22:10

@tlongwell-block tlongwell-block left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Reviewed line-by-line against deployed a35771f and my independent fix. The patch removes both reachable cross-map guard nestings, covers all five fan-out indexes, preserves stale-index safety via the existing subs re-check, and includes the concurrent fan-out/replacement regression test. No blocking findings. GitHub will not let this shared account approve its own PR.

@tlongwell-block tlongwell-block left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

BLOCKER at head 7d2da0d: snapshot candidate for channel A, then same-ID replacement onto channel B, then push_match resumes. push_match reads the new record but validates only filters, not stored community/channel scope, so a kind-only replacement on B receives A's event. Deterministic injected test expected [] and delivered one match. Please pass expected community into push_match and require current record scope == (fanout community, event.channel_id) before filters_match; retain this stale-snapshot test for all snapshot safety. Shared GitHub account prevents REQUEST_CHANGES review state; this comment is a merge blocker.

npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 and others added 3 commits July 28, 2026 18:26
Co-authored-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@buzz.block.builderlab.xyz>
Signed-off-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@buzz.block.builderlab.xyz>
…-lock-order

* origin/main:
  fix(ci): ratchet file sizes against the base tree (#3352)

Signed-off-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@buzz.block.builderlab.xyz>
Co-authored-by: npub1jh9wn95s0472h86ahapupaf7m6kx4v9sx2n0atj2hltcfer8k06s5n3pyf <95cae996907d7cab9f5dbf43c0f53edeac6ab0b032a6feae4abfd784e467b3f5@buzz.block.builderlab.xyz>
Signed-off-by: npub1jh9wn95s0472h86ahapupaf7m6kx4v9sx2n0atj2hltcfer8k06s5n3pyf <95cae996907d7cab9f5dbf43c0f53edeac6ab0b032a6feae4abfd784e467b3f5@buzz.block.builderlab.xyz>
@tlongwell-block
tlongwell-block merged commit 22be8bb into main Jul 29, 2026
32 checks passed
@tlongwell-block
tlongwell-block deleted the wren/fix-subscription-lock-order branch July 29, 2026 01:13
atishpatel added a commit that referenced this pull request Jul 29, 2026
…ounting-and-solo

* origin/main:
  fix(acp): per-runtime env defaults at spawn — isolate Hermes from configured MCP startup (#3420)
  fix(relay): avoid subscription lock inversion (#3413)
  feat: add explicit entry for claude-opus-5 in model config (#2831)
  fix(desktop): clear stale thread new-message pill (#3411)
  fix(ci): ratchet file sizes against the base tree (#3352)
  chore(ci): bump desktop smoke E2E timeout to 30 minutes (#3409)
  release(chart): publish 0.1.7 (#3393)

Signed-off-by: Atish Patel <atish@squareup.com>
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.

1 participant