ionic-team / ionic-team/capacitor
[Bug]: CAPPlugin Event Handling Causing Data Races on iOS
- Dominant language
- TypeScript
- Stars
- 16.7k
- Forks
- 1.3k
- Avg merge
- 3d 15h
- Merged PRs (30d)
- 10
Description
### 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
```Shell
```
### Platforms Affected
- [x] 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:
```swift
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()
}
}
```
Contributor guide
Assessment
This issue has not been assessed yet.