electric-sql / electric-sql/electric
Race condition in drop_replication_slot_on_stop: cast may not arrive before terminate
- Dominant language
- TypeScript
- Stars
- 10.4k
- Forks
- 375
- Avg merge
- 3d 1h
- Merged PRs (30d)
- 18
Description
## Summary
There is a race condition in `Electric.Connection.Manager.drop_replication_slot_on_stop/1` where the cast message may not be processed before the process terminates, causing the replication slot to not be dropped.
## The race
In `packages/sync-service/lib/electric/connection/manager.ex`:
1. **Line 122**: State initializes with `drop_slot_requested: false`
2. **Line 189**: `drop_replication_slot_on_stop(manager)` does `GenServer.cast(manager, :drop_replication_slot_on_stop)` — returns immediately
3. **Line 680**: `handle_cast(:drop_replication_slot_on_stop, state)` flips `drop_slot_requested: true`
4. **Line 874**: `terminate/2` reads `state.drop_slot_requested` and conditionally calls `drop_publication/1` and `drop_slot/1`
The caller (e.g., `TenantManager.do_stop_tenant`) calls `drop_replication_slot_on_termination` (a cast → returns immediately) and *immediately* calls `DynamicTenantSupervisor.stop_tenant` which invokes `DynamicSupervisor.terminate_child`.
**The problem:** The cast and the supervisor's terminate signal are sent from *two different processes* (the TenantManager GenServer vs. the DynamicSupervisor). Erlang's mailbox preserves send order only for messages between the *same* sender pair. There is no happens-before between messages from different senders.
If the supervisor's terminate signal arrives before the cast is processed, `terminate/2` runs with `drop_slot_requested: false` and **the slot is NOT dropped**.
## Impact
The existing manual `clean?: true` path does this same dance today. Either:
- (a) In practice the cast usually wins because of typical scheduling latencies, or
- (b) The manual path silently misses slot drops sometimes and nobody noticed (operators just retry)
This becomes more critical for automated slot cleanup (e.g., when WAL retention exceeds thresholds) where silent failures are harder to detect.
## Proposed fix
Add a synchronous variant `drop_replication_slot_on_stop_sync/1` using `GenServer.call` that flips the flag and returns only after the cast is processed:
```elixir
@spec drop_replication_slot_on_stop_sync(GenServer.server()) :: :ok
def drop_replication_slot_on_stop_sync(manager) do
GenServer.call(manager, :drop_replication_slot_on_stop)
end
def handle_call(:drop_replication_slot_on_stop, _from, state) do
{:reply, :ok, %{state | drop_slot_requested: true}}
end
```
This ensures the flag is set before the caller proceeds to terminate the process.
## References
- Investigation from alco-agent-tasks#37 (section 1.8)
- File: `packages/sync-service/lib/electric/connection/manager.ex`
Contributor guide
Assessment
This issue has not been assessed yet.