Skip to content

fix: eliminate race condition in TestBroadcastEvent_Delivery#6

Merged
darkliquid merged 3 commits intofeat/protojsonfrom
copilot/sub-pr-3-another-one
Mar 15, 2026
Merged

fix: eliminate race condition in TestBroadcastEvent_Delivery#6
darkliquid merged 3 commits intofeat/protojsonfrom
copilot/sub-pr-3-another-one

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 15, 2026

TestBroadcastEvent_Delivery was flaky under load: BroadcastEvent fired immediately after dialUnixWithRetry returned, before the server's serve goroutine had processed Accept()+trackConn(). The broadcast hit an empty conns map and was lost, causing a 5-second timeout.

Fix

  • After dialing, poll srv.conns until the server has registered the connection before calling BroadcastEvent
for {
    srv.mu.Lock()
    tracked := len(srv.conns)
    srv.mu.Unlock()
    if tracked > 0 {
        break
    }
    select {
    case <-ctx.Done():
        t.Fatal("timed out waiting for server to track connection")
    case <-time.After(5 * time.Millisecond):
    }
}

Deterministic and no arbitrary sleeps — package ipc test access to unexported fields makes this straightforward.


📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.

Copilot AI self-assigned this Mar 15, 2026
Co-authored-by: darkliquid <31256+darkliquid@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix IPC JSON-RPC unification issues fix: eliminate race condition in TestBroadcastEvent_Delivery Mar 15, 2026
Copilot AI requested a review from darkliquid March 15, 2026 18:44
@darkliquid darkliquid marked this pull request as ready for review March 15, 2026 18:47
Copilot AI review requested due to automatic review settings March 15, 2026 18:47
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Stabilizes TestBroadcastEvent_Delivery in internal/ipc by ensuring the server has registered the newly-dialed connection before broadcasting, preventing occasional lost broadcasts due to a connection-tracking race.

Changes:

  • Add a polling wait after dialing to ensure srv.conns contains the connection before calling BroadcastEvent.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/ipc/extra_test.go
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@darkliquid darkliquid merged commit 4780ce5 into feat/protojson Mar 15, 2026
darkliquid added a commit that referenced this pull request Mar 15, 2026
* feat: unify IPC JSON-RPC

* fix: add missing js

* fix: add missing files

* ipc: enforce max frame size on write path (#4)

* Initial plan

* fix: add max frame size check in WriteEnvelope

Co-authored-by: darkliquid <31256+darkliquid@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: darkliquid <31256+darkliquid@users.noreply.github.com>

* ipc: address post-review feedback on unified JSON-RPC layer (#5)

* Initial plan

* fix: address all IPC review feedback - naming, forward-compat, connection-drop cleanup, and broadcast tests

Co-authored-by: darkliquid <31256+darkliquid@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Update internal/ipc/extra_test.go

* Update internal/ipc/extra_test.go

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: darkliquid <31256+darkliquid@users.noreply.github.com>
Co-authored-by: Andrew Montgomery <darkliquid@darkliquid.co.uk>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix: eliminate race condition in TestBroadcastEvent_Delivery (#6)

* Initial plan

* fix: resolve race condition in TestBroadcastEvent_Delivery

Co-authored-by: darkliquid <31256+darkliquid@users.noreply.github.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: darkliquid <31256+darkliquid@users.noreply.github.com>
Co-authored-by: Andrew Montgomery <darkliquid@darkliquid.co.uk>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: darkliquid <31256+darkliquid@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.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.

3 participants