[Bug]: CAPPlugin Event Handling Causing Data Races on iOS

Capacitor Version

Capacitor 6.1.1

Though this is reported on 6.1.1 as that's the version we have in our codebase and not the latest 7.x release, as best as I can tell, the code around CAPPlugin's handling of event listeners hasn't changed between those major versions, so I believe that 7.x is affected as well.

Other API Details

Platforms Affected

• iOS
• Android
• Web

Current Behavior

We've noticed that we get crashes sometimes when sending events on the main thread. I can't share the entire stack trace, but the two frames at the top of the crash report are:

0  CoreFoundation                 0x12c0c -[__NSDictionaryM objectForKey:] + 136
1  Capacitor                      0x917c -[CAPPlugin notifyListeners:data:retainUntilConsumed:] + 83 (CAPPlugin.m:83)

My current working theory is that there is some mutation that's happening on a background thread at the same time as calling the plugin to send an event. The eventListeners property of CAPPlugin itself isn't guarded by a lock or any other synchronization mechanism, but it is mutated on CapacitorBridge's dispatchQueue for the various methods that the JS side calls to add or remove a listener. However, there's no direct way to get access to that dispatch queue if I wanted to synchronize sending events to that queue, nor is the -notifyListeners:data:retainUntilConsumed: method run on that dispatch queue, so I've got to go through some additional steps if I want to try to enforce some data safety.

CAPBridgeViewController's bridge property is typed as CAPBridgeProtocol, but dispatchQueue isn't a property on the protocol. I could check the type of the class returned by bridge and cast that so I could get access that property, but if you all change the type of the thing returned from the bridge property, then my potential work around to get access to dispatchQueue to synchronize sending events stops working.

This crash happening can be inconsistent as you might expect for a data race. I've attached a test that exhibits this behavior in the "Additional Info" section, but it can sometimes run without encountering an issue, just with the nature of this being a timing sensitive crash. Utilizing Xcode's functionality to run a test repeatedly helps here to get it to crash.

Expected Behavior

The app would not crash.

Project Reproduction

See "Additional Information" for a test that exhibits the issue.

Additional Information

A test case that exercises the data race:

import Capacitor
import XCTest

final class CAPPluginSynchronizationTest: XCTestCase {
    func testConcurrentAccess() {
        let plugin = CAPPlugin()
        // Need to assign this since it's not assigned in CAPPlugin's initializer, otherwise we're just messaging `nil` everywhere and not actually showcasing the data race.
        plugin.eventListeners = [:]
        
        let group = DispatchGroup()
        let iterations = 100_000
        
        group.enter()
        Thread.detachNewThread {
            for _ in 0 ..< iterations {
                let call = CAPPluginCall(callbackId: UUID().uuidString) { _, _ in
                    
                } error: { _ in
                    
                }!
                
                call.options["eventName"] = "event-name"

                plugin.addListener(call)
            }
            
            group.leave()
        }
        
        group.enter()
        Thread.detachNewThread {
            for _ in 0 ..< iterations {
                let call = CAPPluginCall(callbackId: "") { _, _ in
                    
                } error: { _ in
                    
                }!
                
                plugin.removeAllListeners(call)
            }
            
            group.leave()
        }
        
        group.wait()
    }
}