Skip to content

FakeMultipeerService.sentMessages is an unsynchronized array (latent TSan race) #162

Description

@darioalessandro

RemoteCamTests/SessionTestSupport.swift:25

var sentMessages: [(msg: Message, peers: [MCPeerID], mode: MCSessionSendDataMode)] = []
...
sentMessages.append((msg, peers, mode))   // :44 — from the coordinator's actor context

Appended from the coordinator's actor while tests read it from the test thread — the same data race that made LoopbackMultipeerService.sentMessages crash 5 of 10 runs under Thread Sanitizer (fixed in PR #157, commit 4d81746).

This one isn't what TSan currently trips on, so it was left alone deliberately — but it's the same latent shape and will bite as soon as a test reads it while the actor is still sending.

Fix (same shape as the one already landed)

private let sentMessagesStorage = Locked<[(msg: Message, peers: [MCPeerID], mode: MCSessionSendDataMode)]>([])
var sentMessages: [(msg: Message, peers: [MCPeerID], mode: MCSessionSendDataMode)] {
    get { sentMessagesStorage.value }
    set { sentMessagesStorage.value = newValue }
}

plus sentMessagesStorage.mutate { $0.append(...) } in send. Keeping the property's name and type means all existing reads and removeAll() calls compile untouched (~14 sites in RemoteCamSessionTests).

Locked<T> is in RemoteCam/Locked.swift, reachable from tests via @testable import RemoteShutter.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions