Skip to content

Commit ae002c7

Browse files
SelaseKaySelaseKay
authored
fix(firestore, android): clean up Android transaction listeners on completion (#18475)
* fix(firestore, android): clean up Android transaction listeners on completion * fix(firestore): prevent duplicate transaction completions and handle null transaction listeners
1 parent fb20da9 commit ae002c7

4 files changed

Lines changed: 89 additions & 10 deletions

File tree

‎packages/cloud_firestore/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/firestore/FlutterFirebaseFirestorePlugin.java‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,28 @@ private String registerEventChannel(String prefix, String identifier, StreamHand
277277
return identifier;
278278
}
279279

280+
private void removeEventListener(String identifier) {
281+
synchronized (eventChannels) {
282+
EventChannel eventChannel = eventChannels.remove(identifier);
283+
if (eventChannel != null) {
284+
eventChannel.setStreamHandler(null);
285+
}
286+
}
287+
288+
synchronized (streamHandlers) {
289+
StreamHandler streamHandler = streamHandlers.remove(identifier);
290+
if (streamHandler != null) {
291+
streamHandler.onCancel(null);
292+
}
293+
}
294+
}
295+
296+
private void removeTransaction(String transactionId) {
297+
transactions.remove(transactionId);
298+
removeEventListener(transactionId);
299+
transactionHandlers.remove(transactionId);
300+
}
301+
280302
private void removeEventListeners() {
281303
synchronized (eventChannels) {
282304
for (String identifier : eventChannels.keySet()) {
@@ -292,6 +314,7 @@ private void removeEventListeners() {
292314
streamHandlers.clear();
293315
}
294316

317+
transactions.clear();
295318
transactionHandlers.clear();
296319
}
297320

@@ -560,6 +583,7 @@ public void transactionCreate(
560583
final TransactionStreamHandler handler =
561584
new TransactionStreamHandler(
562585
transaction -> transactions.put(transactionId, transaction),
586+
this::removeTransaction,
563587
firestore,
564588
transactionId,
565589
timeout,
@@ -576,8 +600,13 @@ public void transactionStoreResult(
576600
@NonNull GeneratedAndroidFirebaseFirestore.InternalTransactionResult resultType,
577601
@Nullable List<GeneratedAndroidFirebaseFirestore.InternalTransactionCommand> commands,
578602
@NonNull GeneratedAndroidFirebaseFirestore.VoidResult result) {
579-
Objects.requireNonNull(transactionHandlers.get(transactionId))
580-
.receiveTransactionResponse(resultType, commands);
603+
OnTransactionResultListener handler = transactionHandlers.get(transactionId);
604+
if (handler == null) {
605+
result.success();
606+
return;
607+
}
608+
609+
handler.receiveTransactionResponse(resultType, commands);
581610
result.success();
582611
}
583612

‎packages/cloud_firestore/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/firestore/streamhandler/TransactionStreamHandler.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,13 @@ public interface OnTransactionStartedListener {
3838
void onStarted(Transaction transaction);
3939
}
4040

41+
/** Callback when the transaction has reached a terminal state. */
42+
public interface OnTransactionCompleteListener {
43+
void onComplete(String transactionId);
44+
}
45+
4146
final OnTransactionStartedListener onTransactionStartedListener;
47+
final OnTransactionCompleteListener onTransactionCompleteListener;
4248
final FirebaseFirestore firestore;
4349
final String transactionId;
4450
final Long timeout;
@@ -47,11 +53,13 @@ public interface OnTransactionStartedListener {
4753

4854
public TransactionStreamHandler(
4955
OnTransactionStartedListener onTransactionStartedListener,
56+
OnTransactionCompleteListener onTransactionCompleteListener,
5057
FirebaseFirestore firestore,
5158
String transactionId,
5259
Long timeout,
5360
Long maxAttempts) {
5461
this.onTransactionStartedListener = onTransactionStartedListener;
62+
this.onTransactionCompleteListener = onTransactionCompleteListener;
5563
this.firestore = firestore;
5664
this.transactionId = transactionId;
5765
this.timeout = timeout;
@@ -178,6 +186,7 @@ public void onListen(Object arguments, EventSink events) {
178186
() -> {
179187
events.success(map);
180188
events.endOfStream();
189+
onTransactionCompleteListener.onComplete(transactionId);
181190
});
182191
});
183192
}

‎packages/cloud_firestore/cloud_firestore/example/integration_test/transaction_e2e.dart‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,35 @@ void runTransactionTests() {
213213
expect(snapshot2.data()!['test'], equals('value4'));
214214
});
215215

216+
test(
217+
'runs many sequential transactions with large payloads',
218+
() async {
219+
DocumentReference<Map<String, dynamic>> doc =
220+
await initializeTest('transaction-cleanup-stress');
221+
final payload = <String, Object?>{
222+
for (var i = 0; i < 100; i++) 'field_$i': 'x' * 100,
223+
};
224+
225+
await doc.set({'count': 0, ...payload});
226+
227+
for (var i = 0; i < 100; i++) {
228+
await firestore.runTransaction((transaction) async {
229+
final snapshot = await transaction.get(doc);
230+
final count = snapshot.data()!['count'] as int;
231+
232+
transaction.update(doc, {
233+
'count': count + 1,
234+
...payload,
235+
});
236+
});
237+
}
238+
239+
final snapshot = await doc.get();
240+
expect(snapshot.data()!['count'], 100);
241+
},
242+
skip: kIsWeb,
243+
);
244+
216245
test(
217246
'should abort if timeout is exceeded',
218247
() async {

‎packages/cloud_firestore/cloud_firestore_platform_interface/lib/src/method_channel/method_channel_firestore.dart‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -246,16 +246,20 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform {
246246
).listen(
247247
(event) async {
248248
if (event['error'] != null) {
249-
completer.completeError(
250-
FirebaseException(
251-
plugin: 'cloud_firestore',
252-
code: event['error']['code'],
253-
message: event['error']['message'],
254-
),
255-
);
249+
if (!completer.isCompleted) {
250+
completer.completeError(
251+
FirebaseException(
252+
plugin: 'cloud_firestore',
253+
code: event['error']['code'],
254+
message: event['error']['message'],
255+
),
256+
);
257+
}
256258
return;
257259
} else if (event['complete'] == true) {
258-
completer.complete(result);
260+
if (!completer.isCompleted) {
261+
completer.complete(result);
262+
}
259263
return;
260264
}
261265

@@ -271,6 +275,10 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform {
271275
try {
272276
result = await transactionHandler(transaction) as T;
273277
} catch (error, stack) {
278+
if (completer.isCompleted) {
279+
return;
280+
}
281+
274282
// Signal native that a user error occurred, and finish the
275283
// transaction
276284
await pigeonChannel.transactionStoreResult(
@@ -286,6 +294,10 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform {
286294
return;
287295
}
288296

297+
if (completer.isCompleted) {
298+
return;
299+
}
300+
289301
// Send the transaction commands to Dart.
290302
await pigeonChannel.transactionStoreResult(
291303
transactionId,

0 commit comments

Comments
 (0)