Fix stale PID race in ConsumerRegistry.unregister_name/1

[alco]

Summary

• Replace the no-op unregister_name/1 with an atomic :ets.match_delete/2 that removes the ETS entry only if the pid matches the calling (dying) process
• A naive lookup-then-delete approach is still racey because the entry can be overwritten between the two ETS calls; match_delete is atomic
• Add unit tests covering: basic cleanup, skip-if-different-pid, and the full race scenario (old process unregisters after replacement has registered)

Closes #3864

Test plan

• mix test test/electric/shapes/consumer_registry_test.exs — 16 tests pass (3 new)
• Full test suite passes in CI

🤖 Generated with Claude Code

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@electric-sql/react@3979

npm i https://pkg.pr.new/@electric-sql/client@3979

npm i https://pkg.pr.new/@electric-sql/y-electric@3979

commit: ebdb61c

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.74%. Comparing base (0e376c4) to head (537c8dd).
⚠️ Report is 5 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #3979       +/-   ##
===========================================
+ Coverage   75.75%   88.74%   +12.98%     
===========================================
  Files          11       25       +14     
  Lines         693     2425     +1732     
  Branches      172      612      +440     
===========================================
+ Hits          525     2152     +1627     
- Misses        167      271      +104     
- Partials        1        2        +1     

Flag	Coverage Δ
packages/experimental	87.73% <ø> (ø)
packages/react-hooks	86.48% <ø> (ø)
packages/start	82.83% <ø> (ø)
packages/typescript-client	93.93% <ø> (?)
packages/y-electric	56.05% <ø> (ø)
typescript	88.74% <ø> (+12.98%)	⬆️
unit-tests	88.74% <ø> (+12.98%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

Claude Code Review

Summary

This PR fixes the ConsumerRegistry.unregister_name/1 race condition by replacing the deliberate no-op with an atomic :ets.match_delete/2 call, preventing stale PID removal of a replacement consumer's ETS entry. The PR is in excellent shape and all previous review issues have been addressed.

What's Working Well

• Atomic fix: The match_delete approach is strictly better than the issue's proposed two-step lookup+delete, eliminating the TOCTOU window entirely.
• self() correctness: Called in the dying process context by OTP cleanup machinery, so self() correctly identifies the dying PID — and the comment makes this explicit.
• Double-removal safety: For {:shutdown, :cleanup} and {:shutdown, :suspend} paths, remove_consumer already deletes the ETS entry before unregister_name fires. match_delete on a missing key is a no-op, so there's no double-removal crash risk.
• Clean diff: The PR now only touches the three relevant files — the race fix, its tests, and the changeset. No unrelated noise.
• Test coverage: The three new tests cover basic cleanup, skip-if-different-pid, and the exact race scenario (old process unregisters after replacement has registered). Clear and well-structured.

Issues Found

No outstanding issues.

Issue Conformance

Issue #3864 is well-specified with root cause analysis, three affected code paths, and a proposed fix. This PR implements a strictly better version of the proposed fix (atomic match_delete vs. the TOCTOU-prone lookup+delete). Scope is appropriately narrow. Code path #3 from the issue (restore_shape_and_dependencies/4 dead-PID trust) was noted as a suggestion in the previous review — it remains intentionally deferred per the PR scope, which is fine.

Previous Review Status

All issues resolved:

• Issue Working with Postgres WAL format from Elixir #1 (error message regression in consumer.ex:124): Resolved by removing the consumer.ex changes from this PR — those changes are no longer included in the diff.
• Issue [Merged on #3] Write inserts, updates and deletes to Vaxine #2 (missing changeset entry): Fixed — .changeset/fix-stale-consumer-registry.md has been added with a clear description.
• Suggestion Read from vaxine stream, send transactions to slot server #4 (unrelated files inflating diff): Fixed — the diff is now clean.

---

Review iteration: 2 | 2026-03-16

[blacksmith-sh]

Found 1 test failure on Blacksmith runners:

Failure

Test	View Logs
Elixir.Electric.Replication.PublicationManagerTest/ test component restarts handles relation tracker restart	View Logs

[magnetised]

@alco the reason for the no-op unregister call was to handle race conditions between the consumer registry and the shape log collector. If you just remove the registry entry when the pid terminates but before you remove the shape from the SLC, then the SLC could receive an operation for that shape, see that the consumer process isn't there and start a new process for it. Then the SLC gets the "remove shape" for that new process and you're left with an orphan process.