Skip to content
This repository was archived by the owner on Apr 26, 2026. It is now read-only.

Commit f14d652

Browse files
hyochanclaude
hyochan
and
claude
authored
fix(android,ios): add thread safety to listener operations (#3157)
## Summary - iOS: Apply `NSLock` + snapshot pattern to all listener arrays for thread safety - Android: Apply snapshot pattern to `userChoiceBilling` / `developerProvidedBilling` listeners for consistency - Add thread safety unit tests and multi-listener registration/removal tests ## Changes ### iOS (`ios/HybridRnIap.swift`) - Add `listenerLock` (`NSLock`) for synchronized access to listener arrays - Wrap all add/remove listener methods (`addPurchaseUpdatedListener`, `addPurchaseErrorListener`, `addPromotedProductListenerIOS`, and their remove counterparts) with `listenerLock.withLock` - `sendPurchaseUpdate`, `sendPurchaseError`: iterate over snapshot copy instead of live array (matches Android pattern) - `sendPurchaseError`: protect error dedup state (`lastPurchaseErrorKey`, `lastPurchaseErrorTimestamp`) inside lock since they are accessed from multiple threads - Apply snapshot pattern to promoted product listener iteration - `cleanupExistingState`: clear listener arrays and reset error dedup state inside lock ### Android (`android/.../HybridRnIap.kt`) - `sendUserChoiceBilling`, `sendDeveloperProvidedBilling`: change from direct iteration to snapshot pattern - `endConnection`: add `synchronized` for clearing `userChoiceBillingListenersAndroid` and `developerProvidedBillingListenersAndroid` ### Tests - **Android**: `ListenerThreadSafetyTest.kt` (new) - 5 tests verifying concurrent add/remove/iterate safety - **TypeScript**: `index.test.ts` - 4 new tests verifying multi-listener registration/removal independence ## Test plan - [x] `yarn typecheck` passes - [x] `yarn lint` passes - [x] `yarn test` passes (251 tests, 12 suites) - [x] Pre-commit hooks pass Closes #3150 🤖 Generated with [Claude Code](https://claude.ai/code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Bug Fixes** * Enhanced thread-safety mechanisms for in-app purchase event listeners on iOS and Android to prevent potential crashes when listeners are added, removed, or notified concurrently. * **Tests** * Added comprehensive test suite validating thread-safe listener operations, snapshot semantics, and reliable event delivery across multiple concurrent listeners. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent cc4de64 commit f14d652

4 files changed

Lines changed: 317 additions & 58 deletions

File tree

‎android/src/main/java/com/margelo/nitro/iap/HybridRnIap.kt‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -258,8 +258,8 @@ class HybridRnIap : HybridRnIapSpec() {
258258
synchronized(purchaseUpdatedListeners) { purchaseUpdatedListeners.clear() }
259259
synchronized(purchaseErrorListeners) { purchaseErrorListeners.clear() }
260260
promotedProductListenersIOS.clear()
261-
userChoiceBillingListenersAndroid.clear()
262-
developerProvidedBillingListenersAndroid.clear()
261+
synchronized(userChoiceBillingListenersAndroid) { userChoiceBillingListenersAndroid.clear() }
262+
synchronized(developerProvidedBillingListenersAndroid) { developerProvidedBillingListenersAndroid.clear() }
263263
initDeferred = null
264264
RnIapLog.result("endConnection", true)
265265
true
@@ -1592,9 +1592,8 @@ class HybridRnIap : HybridRnIapSpec() {
15921592
}
15931593

15941594
private fun sendUserChoiceBilling(details: UserChoiceBillingDetails) {
1595-
synchronized(userChoiceBillingListenersAndroid) {
1596-
userChoiceBillingListenersAndroid.forEach { it(details) }
1597-
}
1595+
val snapshot = synchronized(userChoiceBillingListenersAndroid) { ArrayList(userChoiceBillingListenersAndroid) }
1596+
snapshot.forEach { it(details) }
15981597
}
15991598

16001599
// Developer Provided Billing listener (External Payments - 8.3.0+)
@@ -1611,9 +1610,8 @@ class HybridRnIap : HybridRnIapSpec() {
16111610
}
16121611

16131612
private fun sendDeveloperProvidedBilling(details: DeveloperProvidedBillingDetailsAndroid) {
1614-
synchronized(developerProvidedBillingListenersAndroid) {
1615-
developerProvidedBillingListenersAndroid.forEach { it(details) }
1616-
}
1613+
val snapshot = synchronized(developerProvidedBillingListenersAndroid) { ArrayList(developerProvidedBillingListenersAndroid) }
1614+
snapshot.forEach { it(details) }
16171615
}
16181616

16191617
// -------------------------------------------------------------------------

‎android/src/test/java/com/margelo/nitro/iap/ListenerThreadSafetyTest.kt‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
package com.margelo.nitro.iap
2+
3+
import org.junit.Test
4+
import org.junit.Assert.*
5+
import java.util.concurrent.CyclicBarrier
6+
import java.util.concurrent.atomic.AtomicInteger
7+
import java.util.concurrent.atomic.AtomicReference
8+
import kotlin.concurrent.thread
9+
10+
/**
11+
* Thread safety tests for the synchronized + snapshot listener pattern
12+
* used in HybridRnIap to prevent ConcurrentModificationException.
13+
*
14+
* Addresses Issue #3150 where purchase events were silently lost
15+
* due to concurrent listener access.
16+
*/
17+
class ListenerThreadSafetyTest {
18+
19+
@Test
20+
fun `concurrent add and snapshot iterate does not throw`() {
21+
val listeners = mutableListOf<(String) -> Unit>()
22+
val callCount = AtomicInteger(0)
23+
val errorRef = AtomicReference<Throwable?>(null)
24+
val iterations = 500
25+
val barrier = CyclicBarrier(2)
26+
27+
val adder = thread {
28+
barrier.await()
29+
repeat(iterations) {
30+
synchronized(listeners) { listeners.add { callCount.incrementAndGet() } }
31+
}
32+
}
33+
34+
val sender = thread {
35+
barrier.await()
36+
repeat(iterations) {
37+
val snapshot = synchronized(listeners) { ArrayList(listeners) }
38+
snapshot.forEach {
39+
try {
40+
it("event")
41+
} catch (e: Throwable) {
42+
errorRef.compareAndSet(null, e)
43+
}
44+
}
45+
}
46+
}
47+
48+
adder.join(5000)
49+
sender.join(5000)
50+
assertFalse("Adder thread did not finish", adder.isAlive)
51+
assertFalse("Sender thread did not finish", sender.isAlive)
52+
assertNull("Should not throw ConcurrentModificationException", errorRef.get())
53+
}
54+
55+
@Test
56+
fun `concurrent add, remove, and iterate is safe`() {
57+
val listeners = mutableListOf<(String) -> Unit>()
58+
val errorRef = AtomicReference<Throwable?>(null)
59+
val barrier = CyclicBarrier(3)
60+
61+
val adder = thread {
62+
barrier.await()
63+
repeat(200) {
64+
synchronized(listeners) { listeners.add { _ -> } }
65+
}
66+
}
67+
68+
val remover = thread {
69+
barrier.await()
70+
repeat(200) {
71+
synchronized(listeners) {
72+
if (listeners.isNotEmpty()) listeners.removeAt(0)
73+
}
74+
}
75+
}
76+
77+
val sender = thread {
78+
barrier.await()
79+
repeat(200) {
80+
val snapshot = synchronized(listeners) { ArrayList(listeners) }
81+
snapshot.forEach {
82+
try {
83+
it("event")
84+
} catch (e: Throwable) {
85+
errorRef.compareAndSet(null, e)
86+
}
87+
}
88+
}
89+
}
90+
91+
adder.join(5000)
92+
remover.join(5000)
93+
sender.join(5000)
94+
assertFalse("Adder thread did not finish", adder.isAlive)
95+
assertFalse("Remover thread did not finish", remover.isAlive)
96+
assertFalse("Sender thread did not finish", sender.isAlive)
97+
assertNull("Concurrent access should be safe with snapshot pattern", errorRef.get())
98+
}
99+
100+
@Test
101+
fun `snapshot delivers to all registered listeners`() {
102+
val listeners = mutableListOf<(String) -> Unit>()
103+
val results = mutableListOf<String>()
104+
105+
synchronized(listeners) {
106+
listeners.add { results.add("listener1:$it") }
107+
listeners.add { results.add("listener2:$it") }
108+
}
109+
110+
val snapshot = synchronized(listeners) { ArrayList(listeners) }
111+
snapshot.forEach { it("event") }
112+
113+
assertEquals(2, results.size)
114+
assertTrue(results.contains("listener1:event"))
115+
assertTrue(results.contains("listener2:event"))
116+
}
117+
118+
@Test
119+
fun `synchronized clear removes all listeners`() {
120+
val listeners = mutableListOf<(String) -> Unit>()
121+
synchronized(listeners) {
122+
listeners.add { _ -> }
123+
listeners.add { _ -> }
124+
}
125+
126+
synchronized(listeners) { listeners.clear() }
127+
128+
val snapshot = synchronized(listeners) { ArrayList(listeners) }
129+
assertTrue("Listeners should be empty after clear", snapshot.isEmpty())
130+
}
131+
132+
@Test
133+
fun `snapshot is isolated from subsequent modifications`() {
134+
val listeners = mutableListOf<(String) -> Unit>()
135+
val results = mutableListOf<String>()
136+
137+
synchronized(listeners) {
138+
listeners.add { results.add("original:$it") }
139+
}
140+
141+
// Take snapshot before adding more listeners
142+
val snapshot = synchronized(listeners) { ArrayList(listeners) }
143+
144+
// Add another listener after snapshot
145+
synchronized(listeners) {
146+
listeners.add { results.add("added-after:$it") }
147+
}
148+
149+
// Only the original listener should be in the snapshot
150+
snapshot.forEach { it("event") }
151+
assertEquals(1, results.size)
152+
assertEquals("original:event", results[0])
153+
}
154+
}

‎ios/HybridRnIap.swift‎

Lines changed: 65 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@ class HybridRnIap: HybridRnIapSpec {
2323
private var deliveredPurchaseEventOrder: [String] = []
2424
private let purchaseEventDedupLimit = 128
2525
private var purchasePayloadById: [String: [String: Any]] = [:]
26-
26+
// Thread safety lock for listener arrays and error dedup state
27+
private let listenerLock = NSLock()
28+
2729
// MARK: - Initialization
2830

2931
override init() {
@@ -851,8 +853,8 @@ class HybridRnIap: HybridRnIapSpec {
851853
}
852854

853855
func addPromotedProductListenerIOS(listener: @escaping (NitroProduct) -> Void) throws {
854-
promotedProductListeners.append(listener)
855-
856+
listenerLock.withLock { promotedProductListeners.append(listener) }
857+
856858
// If a promoted product is already available from OpenIAP, notify immediately
857859
Task {
858860
RnIapLog.payload("promotedProductListenerIOS.fetch", nil)
@@ -864,33 +866,27 @@ class HybridRnIap: HybridRnIapSpec {
864866
}
865867
}
866868
}
867-
869+
868870
func removePromotedProductListenerIOS(listener: @escaping (NitroProduct) -> Void) throws {
869-
// Note: In Swift, comparing closures is not straightforward, so we'll clear all listeners
870-
// In a real implementation, you might want to use a unique identifier for each listener
871-
promotedProductListeners.removeAll()
871+
listenerLock.withLock { promotedProductListeners.removeAll() }
872872
}
873-
873+
874874
// MARK: - Event Listener Methods
875-
875+
876876
func addPurchaseUpdatedListener(listener: @escaping (NitroPurchase) -> Void) throws {
877-
purchaseUpdatedListeners.append(listener)
877+
listenerLock.withLock { purchaseUpdatedListeners.append(listener) }
878878
}
879-
879+
880880
func addPurchaseErrorListener(listener: @escaping (NitroPurchaseResult) -> Void) throws {
881-
purchaseErrorListeners.append(listener)
881+
listenerLock.withLock { purchaseErrorListeners.append(listener) }
882882
}
883-
883+
884884
func removePurchaseUpdatedListener(listener: @escaping (NitroPurchase) -> Void) throws {
885-
// Note: This is a limitation of Swift closures - we can't easily remove by reference
886-
// For now, we'll just clear all listeners when requested
887-
purchaseUpdatedListeners.removeAll()
885+
listenerLock.withLock { purchaseUpdatedListeners.removeAll() }
888886
}
889-
887+
890888
func removePurchaseErrorListener(listener: @escaping (NitroPurchaseResult) -> Void) throws {
891-
// Note: This is a limitation of Swift closures - we can't easily remove by reference
892-
// For now, we'll just clear all listeners when requested
893-
purchaseErrorListeners.removeAll()
889+
listenerLock.withLock { purchaseErrorListeners.removeAll() }
894890
}
895891

896892
// MARK: - Private Helper Methods
@@ -954,20 +950,22 @@ class HybridRnIap: HybridRnIapSpec {
954950
RnIapLog.result("fetchProducts", payloads)
955951
if let payload = payloads.first {
956952
let nitro = RnIapHelper.convertProductDictionary(payload)
953+
let snapshot = self.listenerLock.withLock { Array(self.promotedProductListeners) }
957954
await MainActor.run {
958-
for listener in self.promotedProductListeners { listener(nitro) }
955+
for listener in snapshot { listener(nitro) }
959956
}
960957
}
961958
} catch {
962959
RnIapLog.failure("promotedProductListenerIOS", error: error)
963960
let id = productId
961+
let snapshot = self.listenerLock.withLock { Array(self.promotedProductListeners) }
964962
await MainActor.run {
965963
var minimal = NitroProduct()
966964
minimal.id = id
967965
minimal.title = id
968966
minimal.type = "inapp"
969967
minimal.platform = .ios
970-
for listener in self.promotedProductListeners { listener(minimal) }
968+
for listener in snapshot { listener(minimal) }
971969
}
972970
}
973971
}
@@ -992,44 +990,59 @@ class HybridRnIap: HybridRnIapSpec {
992990
]
993991
let eventKey = keyComponents.joined(separator: "#")
994992

995-
if deliveredPurchaseEventKeys.contains(eventKey) {
996-
RnIapLog.warn("Duplicate purchase update skipped for \(purchase.productId)")
997-
return
993+
var isDuplicate = false
994+
let snapshot: [(NitroPurchase) -> Void] = listenerLock.withLock {
995+
if deliveredPurchaseEventKeys.contains(eventKey) {
996+
isDuplicate = true
997+
return []
998+
}
999+
1000+
deliveredPurchaseEventKeys.insert(eventKey)
1001+
deliveredPurchaseEventOrder.append(eventKey)
1002+
if deliveredPurchaseEventOrder.count > purchaseEventDedupLimit, let removed = deliveredPurchaseEventOrder.first {
1003+
deliveredPurchaseEventOrder.removeFirst()
1004+
deliveredPurchaseEventKeys.remove(removed)
1005+
}
1006+
1007+
return Array(purchaseUpdatedListeners)
9981008
}
9991009

1000-
deliveredPurchaseEventKeys.insert(eventKey)
1001-
deliveredPurchaseEventOrder.append(eventKey)
1002-
if deliveredPurchaseEventOrder.count > purchaseEventDedupLimit, let removed = deliveredPurchaseEventOrder.first {
1003-
deliveredPurchaseEventOrder.removeFirst()
1004-
deliveredPurchaseEventKeys.remove(removed)
1010+
if isDuplicate {
1011+
RnIapLog.warn("Duplicate purchase update skipped for \(purchase.productId)")
1012+
return
10051013
}
10061014

1007-
for listener in purchaseUpdatedListeners {
1015+
for listener in snapshot {
10081016
listener(purchase)
10091017
}
10101018
}
1011-
1019+
10121020
private func sendPurchaseError(_ error: NitroPurchaseResult, productId: String? = nil) {
1013-
let now = Date().timeIntervalSince1970
10141021
let dedupIdentifier = productId
10151022
?? (error.purchaseToken?.isEmpty == false ? error.purchaseToken : nil)
10161023
?? (error.message.isEmpty ? nil : error.message)
10171024
let currentKey = RnIapHelper.makeErrorDedupKey(code: error.code, productId: dedupIdentifier)
1018-
// Dedup only when the exact same error is emitted almost simultaneously.
1019-
let withinWindow = (now - lastPurchaseErrorTimestamp) < 0.15
1020-
if currentKey == lastPurchaseErrorKey && withinWindow {
1021-
return
1022-
}
10231025

1024-
lastPurchaseErrorKey = currentKey
1025-
lastPurchaseErrorTimestamp = now
1026+
// Protect error dedup state since sendPurchaseError is called from multiple threads
1027+
let shouldSkip: Bool = listenerLock.withLock {
1028+
let now = Date().timeIntervalSince1970
1029+
let withinWindow = (now - lastPurchaseErrorTimestamp) < 0.15
1030+
if currentKey == lastPurchaseErrorKey && withinWindow {
1031+
return true
1032+
}
1033+
lastPurchaseErrorKey = currentKey
1034+
lastPurchaseErrorTimestamp = now
1035+
return false
1036+
}
1037+
if shouldSkip { return }
10261038

10271039
// Ensure we never leak SKU via purchaseToken
10281040
var sanitized = error
10291041
if let pid = productId, sanitized.purchaseToken == pid {
10301042
sanitized.purchaseToken = nil
10311043
}
1032-
for listener in purchaseErrorListeners {
1044+
let snapshot = listenerLock.withLock { Array(purchaseErrorListeners) }
1045+
for listener in snapshot {
10331046
listener(sanitized)
10341047
}
10351048
}
@@ -1067,15 +1080,17 @@ class HybridRnIap: HybridRnIapSpec {
10671080
RnIapLog.result("endConnection", result as Any)
10681081
}
10691082

1070-
// Clear event listeners
1071-
purchaseUpdatedListeners.removeAll()
1072-
purchaseErrorListeners.removeAll()
1073-
promotedProductListeners.removeAll()
1074-
deliveredPurchaseEventKeys.removeAll()
1075-
deliveredPurchaseEventOrder.removeAll()
1076-
purchasePayloadById.removeAll()
1077-
lastPurchaseErrorKey = nil
1078-
lastPurchaseErrorTimestamp = 0
1083+
// Clear event listeners, error dedup state, and delivery state (thread-safe)
1084+
listenerLock.withLock {
1085+
purchaseUpdatedListeners.removeAll()
1086+
purchaseErrorListeners.removeAll()
1087+
promotedProductListeners.removeAll()
1088+
lastPurchaseErrorKey = nil
1089+
lastPurchaseErrorTimestamp = 0
1090+
deliveredPurchaseEventKeys.removeAll()
1091+
deliveredPurchaseEventOrder.removeAll()
1092+
purchasePayloadById.removeAll()
1093+
}
10791094
}
10801095

10811096
func deepLinkToSubscriptionsAndroid(options: NitroDeepLinkOptionsAndroid) throws -> Promise<Void> {

0 commit comments

Comments
 (0)