Fix Purchase.synchronizeReceipts re-submitting same receipt and firing callback multiple times#4990
Merged
Merged
Conversation
…g callback N times Three closely-related bugs surfaced from a forum report of submitReceipt being invoked repeatedly: 1. removePendingPurchase matched only on transactionId and silently no-op'd when the receipt's transactionId was null. The pending queue then still contained the receipt, the recursion at the end of synchronizeReceipts pulled it again, and the same receipt was re-submitted forever. removePendingPurchase now takes the Receipt itself and falls back to matching on (sku, storeCode, purchaseDate, orderData) when transactionId is null on either side. 2. The recursive synchronizeReceipts(0, callback) re-registered the caller's SuccessCallback on every iteration, so a queue of N pending receipts caused the user's callback to fire N times. The recursive call now passes null since the original callback is already in synchronizeReceiptsCallbacks. 3. syncInProgress was reset to false after removePendingPurchase, so a throw from removePendingPurchase (or any user-supplied submitReceipt implementation) permanently wedged the sync state. syncInProgress is now reset at the top of onSucess. Added three regression tests in PurchaseTest: - testSynchronizeReceiptsSyncDrainsMultiplePendingReceipts: each pending receipt is submitted exactly once. - testSynchronizeReceiptsCallbackFiresOnceWhenDrainingMultiplePendingReceipts - testSynchronizeReceiptsDoesNotInfinitelyResubmitReceiptWithNullTransactionId (uses a CountingReceiptStore that throws after a cap so a regression fails the test instead of hanging it). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Collaborator
Author
|
Compared 110 screenshots: 110 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 110 screenshots: 110 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 110 screenshots: 110 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
…r id Until now the framework's contract was effectively "submitReceipt fires once per pending-queue entry," which leaks platform-level behavior to the user: iOS StoreKit can redeliver an unfinished transaction across app sessions, and sandbox subscription renewals fire repeatedly — each delivery hits postReceipt and produces another submitReceipt call with the same transactionId. The user-facing contract was inconsistent across platforms. Add a persistent List<String> of processed transactionIds in CN1 Storage (key ProcessedPurchases.dat). Two checks close the gap: 1. addPendingPurchase drops a receipt whose transactionId is already in the processed set, or already sitting in the pending queue. So duplicate postReceipt calls — from iOS cross-session redelivery, in-session double-fire, or anything else — never reach the queue. 2. The success branch of synchronizeReceipts records the transactionId in the processed set before removing the receipt from pending. Doing it in that order means a parallel re-enqueue racing the remove is also dropped. Receipts with a null transactionId can't be tracked in the set; they fall back to the existing receiptsMatch-based in-pending dedup. Two new tests: - testPostReceiptSkipsReceiptThatWasAlreadySuccessfullySubmitted (cross-session redelivery — verified the test fails 1 vs 2 without the fix) - testPostReceiptSkipsDuplicateTransactionIdAlreadyPending (same-session double-fire before sync runs) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
shai-almog
added a commit
that referenced
this pull request
May 28, 2026
…read (#5064) * Fix #5010: serialize Purchase state on the EDT to unblock iOS main thread PR #4990 introduced extra Storage I/O inside `Purchase.addPendingPurchase` (it now reads ProcessedPurchases.dat in addition to PendingPurchases.dat). That I/O happens inside `synchronized (PENDING_PURCHASE_LOCK)`, on whichever thread native code calls `postReceipt` from -- on iOS that's the main UI thread (StoreKit invokes `paymentQueue:updatedTransactions:` on it, including sandbox auto-renewals that fire on a compressed schedule on iOS 26.x). The added per-callback I/O was enough to wedge keyboard input once a TextField was active, because the same iOS main thread is responsible for both StoreKit delivery and UIKit events. Replace the three lock objects (`PENDING_PURCHASE_LOCK`, `synchronizationLock`, `receiptsLock`) with EDT-based serialization: - All state mutations (`addPendingPurchase`, `removePendingPurchase`, `recordProcessedTransactionId`, `setReceipts`, `setReceiptsRefreshTime`, `synchronizeReceipts`, `loadReceipts`, `postReceipt`) auto-dispatch to the EDT via `Display.callSerially` when called off-EDT. Once on the EDT, single-threaded execution makes `synchronized` blocks unnecessary. - `ReceiptStore.submitReceipt` / `fetchReceipts` callbacks may fire on any thread; the new `onSubmitReceiptComplete` / `onLoadReceiptsComplete` helpers (and the inline `loadReceipts` fetch callback) re-dispatch to the EDT before touching state, so reentrancy from any thread is safe. - Public read accessors (`getReceipts`, `getPendingPurchases`) preserve their existing "callable from any thread" contract by using `callSeriallyAndWait` when invoked off-EDT. - `receiptStore` is marked `volatile` for safe publication, since `setReceiptStore` / `getReceiptStore` / the `subscribe` family have no thread contract. Net effect: - iOS native thread returns from `postReceipt` immediately; Storage I/O no longer blocks the UI thread. - The recursive `synchronizeReceipts(0, null)` becomes a tail `callSerially` instead of a same-thread recursive call -- naturally bounded, no held-lock concerns. - All three lock objects, plus the `synchronized` blocks they guarded, are deleted. The only remaining `synchronized` blocks are on a local `boolean[]` monitor inside `SynchronizeReceiptsSyncRunnable` / `SynchronizeReceiptsSyncSuccessCallback`, used for the existing `invokeAndBlock` wait/notify pattern in `synchronizeReceiptsSync`. All 16 tests in `PurchaseTest` pass unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Eagerly initialize synchronizeReceiptsCallbacks to silence SpotBugs The lazy-init pattern was acceptable while the field was protected by synchronizationLock, but after the EDT-serialization refactor SpotBugs flags it as LI_LAZY_INIT_STATIC because the write to the static field is unsynchronized. Access is EDT-only by construction so there is no real race, but the simplest way to silence the analyzer is to drop the lazy init entirely: declare the list final, initialize it at class load, and remove the now-redundant null guards in synchronizeReceipts and fireSynchronizeReceiptsCallbacks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix PMD: drop volatile on receiptStore, add @OverRide to anonymous Runnables Two PMD rules fired on the EDT-serialization refactor: - AvoidUsingVolatile on `receiptStore`. The field was not volatile before this refactor; I had added the modifier defensively for safe publication across the off-EDT setter/getter. The pre-existing behavior (plain field) was unchanged for years, so drop the modifier to stay within the project's coding standards. - MissingOverride on the nine new anonymous `Runnable` instances added by the auto-dispatch pattern. Add the @OverRide annotation to each. No behavior change. PurchaseTest 16/16 still pass; local SpotBugs and PMD both clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Forum report: a user implementing
ReceiptStore(storage-style —submitReceiptposts to their server and callscallback.onSucess(true)inline) seessubmitReceiptinvoked many times after a single in-app purchase. Investigation surfaced several bugs and a cross-platform abstraction gap.Commit 1 — Fix three bugs in
synchronizeReceiptsBug 1: receipts with a null
transactionIdare resubmitted forever.removePendingPurchase(String transactionId)only matched receipts with a non-null storedtransactionId. If a receipt with null tx reached the queue, remove was a no-op, the recursion picked it back up, andsubmitReceiptfired forever.removePendingPurchasenow takes theReceiptitself and falls back to matching on(sku, storeCode, purchaseDate, orderData)whentransactionIdis null on either side.Bug 2: caller's
SuccessCallbackfires N times for N pending receipts. The recursivesynchronizeReceipts(0, callback)re-registered the user's callback every iteration. Now passesnullsince the callback is already registered on the top-level call.Bug 3: thrown exceptions permanently wedge
syncInProgress.syncInProgress = falsewas set afterremovePendingPurchase. Any throw left it stucktruefor the rest of the app's lifetime. Now reset at the top ofonSucessbefore any work that can throw.Commit 2 — Dedupe
submitReceiptontransactionIdacross the install lifetimeThe framework's implicit contract was "submitReceipt fires once per pending-queue entry," which leaks platform behavior: iOS StoreKit redelivers unfinished transactions across sessions, sandbox subscription renewals fire on a compressed schedule, etc. — each delivery hits
postReceiptand produces anothersubmitReceiptcall with the sametransactionId.Now backed by a persistent
List<String>of processed transactionIds (Storage keyProcessedPurchases.dat):addPendingPurchasedrops a receipt whosetransactionIdis already in the processed set, or already sitting in the pending queue. DuplicatepostReceiptcalls never reach the queue.transactionIdbeforeremovePendingPurchase, so a parallel re-enqueue racing the remove is also dropped.Receipts with a null
transactionIdcan't be tracked in the set; they fall back to the existingreceiptsMatch-based in-pending dedup added in commit 1.The user-facing contract is now:
submitReceiptis invoked at most once pertransactionIdfor the lifetime of the install, on every platform.Test plan
PurchaseTest:testSynchronizeReceiptsSyncDrainsMultiplePendingReceipts— each pending receipt is submitted exactly once.testSynchronizeReceiptsCallbackFiresOnceWhenDrainingMultiplePendingReceipts— user callback fires once, not N times.testSynchronizeReceiptsDoesNotInfinitelyResubmitReceiptWithNullTransactionId— uses aCountingReceiptStorethat throws after a cap so a regression fails the test instead of hanging it.testPostReceiptSkipsReceiptThatWasAlreadySuccessfullySubmitted— simulates iOS cross-session redelivery.testPostReceiptSkipsDuplicateTransactionIdAlreadyPending— same-session double-fire before sync runs.PurchaseTestpass (mvn test -Dtest=PurchaseTest).submitReceipt invoked more than 5 times; pending receipt is being resubmitted in a loop; cross-session redelivery test failsexpected: <1> but was: <2>without the dedup).🤖 Generated with Claude Code