Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 30, 2025

The snapshot stub's start/0 and stop/0 functions have a race window between ets:info/1 check and ets:new/2/ets:delete/1 operations. Concurrent calls can cause badarg exceptions.

Changes

  • Wrap ETS operations in try/catch: Both ets:new/2 and ets:delete/1 now catch badarg and return ok, treating the race as a success case (table already created/deleted)

  • Add concurrent test coverage: New test module validates idempotent behavior under concurrent access (10-20 processes calling start/stop simultaneously)

start() ->
    case ets:info(?TABLE) of
        undefined ->
            try
                _ = ets:new(?TABLE, [set, named_table, public]),
                ok
            catch
                error:badarg -> ok  % Another process created it
            end;
        _ -> ok
    end.

The operations remain idempotent while handling the edge case gracefully.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • esm.ubuntu.com
    • Triggering command: /usr/lib/apt/methods/https (dns block)
  • repo.hex.pm
    • Triggering command: /home/REDACTED/.asdf/installs/erlang/27.1.2/erts-15.1.2/bin/inet_gethost 4 (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

This section details on the original issue you should resolve

<issue_title>Harden ETS start/stop in snapshot stub against concurrent TOCTOU races</issue_title>
<issue_description>## Context

In the test stub module event_sourcing_core_store_snapshot_stub.erl, the current start/0 and stop/0 implementations use ets:info/1 before ets:new/2 and ets:delete/1, which creates a theoretical TOCTOU (Time-Of-Check-Time-Of-Use) race condition if multiple processes attempt to start or stop the backend concurrently.

Current Behavior

Between the check (ets:info/1) and the action (ets:new/2 or ets:delete/1), another process could create or delete the named table, potentially causing a badarg exception.

Proposed Improvement

Wrap the ets:new/2 and ets:delete/1 calls in try/catch blocks to handle badarg gracefully, making the operations truly idempotent under concurrent access.

Priority

This is a theoretical issue in the current usage, as the backend is not started/stopped from multiple processes. However, it would be good practice to ensure proper concurrent idempotence for robustness.

References

Comments on the Issue (you are @copilot in this section)

@ccamel Not a real issue in tests, but worth fixing.

Note

Custom agent used: erlang_otp_developer
Erlang/OTP Developer with expertise in building fault-tolerant, distributed systems using Erlang and OTP principles in an event-sourced architecture.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@coderabbitai
Copy link

coderabbitai bot commented Oct 30, 2025

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI and others added 2 commits October 30, 2025 15:29
Wrap ets:new/2 and ets:delete/1 calls in try/catch blocks to handle
badarg exceptions gracefully. This prevents potential race conditions
when multiple processes attempt to start or stop the backend concurrently.

Between checking if the table exists (ets:info/1) and creating/deleting
it, another process could modify the table state, causing badarg. The
try/catch ensures these operations are truly idempotent.

Add focused tests for concurrent start/stop scenarios to validate the fix.

Co-authored-by: ccamel <[email protected]>
Fix unused variable warnings in concurrent test list comprehensions.
Replace 'Pid <- Pids' with '_ <- Pids' since we're not matching on
specific process IDs in the receive clauses.

Co-authored-by: ccamel <[email protected]>
Copilot AI changed the title [WIP] Fix TOCTOU races in ETS start/stop methods Fix TOCTOU race in snapshot stub ETS operations Oct 30, 2025
Copilot AI requested a review from ccamel October 30, 2025 15:36
Copilot finished work on behalf of ccamel October 30, 2025 15:36
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.

Harden ETS start/stop in snapshot stub against concurrent TOCTOU races

2 participants