fix: audience occurrence interval limits bypassed under memory pressure#455
fix: audience occurrence interval limits bypassed under memory pressure#455
Conversation
The countAudienceOccurrences and countPlacement methods were delegating to coreDataStack.count() which uses a separate backgroundContext from the one CoreDataManager uses for saving. Under memory pressure, the cross-context merge notification could be delayed, causing the count query to return 0 and bypassing audience interval limits. This fix moves both count methods to use CoreDataManager's own backgroundContext directly, ensuring save and count operate on the same context with immediate visibility. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| func test_saveAndCountAudienceOccurrence_sameContext() async { | ||
| let key = "same_context_test" | ||
| let maxCount = 1 | ||
| let interval: TriggerAudienceOccurrence.Interval = .minutes(60) | ||
| let occurrence = TriggerAudienceOccurrence( | ||
| key: key, | ||
| maxCount: maxCount, | ||
| interval: interval | ||
| ) | ||
|
|
||
| // Save via CoreDataManager (uses its own backgroundContext) | ||
| let saveExpectation = expectation(description: "Saved occurrence") | ||
| coreDataManager.save(triggerAudienceOccurrence: occurrence) { _ in | ||
| saveExpectation.fulfill() | ||
| } | ||
| await fulfillment(of: [saveExpectation], timeout: 2.0) | ||
|
|
||
| // Count should find the saved occurrence on the same context | ||
| let count = await coreDataManager.countAudienceOccurrences(for: occurrence) | ||
| XCTAssertEqual(count, 1, "Count should see the occurrence saved on the same context") | ||
| } | ||
|
|
||
| func test_saveAndCountAudienceOccurrence_infinity() async { | ||
| let key = "infinity_context_test" | ||
| let maxCount = 1 | ||
| let interval: TriggerAudienceOccurrence.Interval = .infinity | ||
| let occurrence = TriggerAudienceOccurrence( | ||
| key: key, | ||
| maxCount: maxCount, | ||
| interval: interval | ||
| ) | ||
|
|
||
| // Save via CoreDataManager | ||
| let saveExpectation = expectation(description: "Saved occurrence") | ||
| coreDataManager.save(triggerAudienceOccurrence: occurrence) { _ in | ||
| saveExpectation.fulfill() | ||
| } | ||
| await fulfillment(of: [saveExpectation], timeout: 2.0) | ||
|
|
||
| // Count should find it immediately | ||
| let count = await coreDataManager.countAudienceOccurrences(for: occurrence) | ||
| XCTAssertEqual(count, 1, "Count should see the occurrence saved on the same context") | ||
| } | ||
|
|
||
| func test_saveMultipleAndCountAudienceOccurrences() async { | ||
| let key = "multi_save_test" | ||
| let maxCount = 5 | ||
| let interval: TriggerAudienceOccurrence.Interval = .minutes(60) | ||
| let occurrence = TriggerAudienceOccurrence( | ||
| key: key, | ||
| maxCount: maxCount, | ||
| interval: interval | ||
| ) | ||
|
|
||
| // Save 3 occurrences via CoreDataManager | ||
| let saveExpectation = expectation(description: "Saved occurrences") | ||
| saveExpectation.expectedFulfillmentCount = 3 | ||
| for _ in 0..<3 { | ||
| coreDataManager.save(triggerAudienceOccurrence: occurrence) { _ in | ||
| saveExpectation.fulfill() | ||
| } | ||
| } | ||
| await fulfillment(of: [saveExpectation], timeout: 2.0) | ||
|
|
||
| // Count should find all 3 | ||
| let count = await coreDataManager.countAudienceOccurrences(for: occurrence) | ||
| XCTAssertEqual(count, 3, "Count should see all occurrences saved on the same context") | ||
| } |
There was a problem hiding this comment.
CLAUDE.md explicitly states:
"This project uses Swift's Testing framework (not XCTest) for all unit tests. Always use the Testing framework when writing new tests."
All three new tests (test_saveAndCountAudienceOccurrence_sameContext, test_saveAndCountAudienceOccurrence_infinity, test_saveMultipleAndCountAudienceOccurrences) use XCTestCase APIs (expectation(description:), await fulfillment(of:), XCTAssertEqual). They should be ported to Swift Testing (@Test, #expect, withKnownIssue, etc.) to stay consistent with the project's testing conventions.
Note: This applies even though the surrounding file already uses XCTest — per the guide, new tests should use Swift Testing.
Rule Used: CLAUDE.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: Tests/SuperwallKitTests/Storage/Core Data/CoreDataManagerTests.swift
Line: 655-722
Comment:
**New tests use XCTest instead of Swift Testing framework**
`CLAUDE.md` explicitly states:
> "This project uses Swift's Testing framework (not XCTest) for all unit tests. Always use the Testing framework when writing new tests."
All three new tests (`test_saveAndCountAudienceOccurrence_sameContext`, `test_saveAndCountAudienceOccurrence_infinity`, `test_saveMultipleAndCountAudienceOccurrences`) use `XCTestCase` APIs (`expectation(description:)`, `await fulfillment(of:)`, `XCTAssertEqual`). They should be ported to Swift Testing (`@Test`, `#expect`, `withKnownIssue`, etc.) to stay consistent with the project's testing conventions.
Note: This applies even though the surrounding file already uses XCTest — per the guide, new tests should use Swift Testing.
**Rule Used:** CLAUDE.md ([source](https://app.greptile.com/review/custom-context?memory=ae0afcf7-0b55-482b-9764-29f361e46714))
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| func test_saveMultipleAndCountAudienceOccurrences() async { | ||
| let key = "multi_save_test" | ||
| let maxCount = 5 | ||
| let interval: TriggerAudienceOccurrence.Interval = .minutes(60) | ||
| let occurrence = TriggerAudienceOccurrence( | ||
| key: key, | ||
| maxCount: maxCount, | ||
| interval: interval | ||
| ) | ||
|
|
||
| // Save 3 occurrences via CoreDataManager | ||
| let saveExpectation = expectation(description: "Saved occurrences") | ||
| saveExpectation.expectedFulfillmentCount = 3 | ||
| for _ in 0..<3 { | ||
| coreDataManager.save(triggerAudienceOccurrence: occurrence) { _ in | ||
| saveExpectation.fulfill() | ||
| } | ||
| } | ||
| await fulfillment(of: [saveExpectation], timeout: 2.0) | ||
|
|
||
| // Count should find all 3 | ||
| let count = await coreDataManager.countAudienceOccurrences(for: occurrence) | ||
| XCTAssertEqual(count, 3, "Count should see all occurrences saved on the same context") | ||
| } |
There was a problem hiding this comment.
maxCount capped by fetchLimit hides the real count
In countAudienceOccurrences, fetchRequest.fetchLimit is set to audienceOccurrence.maxCount (which is 5 in this test), and NSManagedObjectContext.count(for:) respects fetchLimit. This means the test with maxCount = 5 and 3 saved records will return 3 correctly, but if you had saved 6 records, the count would be capped at 5 (the fetchLimit), not 6.
The test only validates the "below limit" case, so it doesn't exercise the cap behaviour. Consider adding a complementary case where maxCount < numberOfSaves to verify the limit is properly enforced (e.g., save 5 occurrences with maxCount = 3 and assert count == 3).
Prompt To Fix With AI
This is a comment left during a code review.
Path: Tests/SuperwallKitTests/Storage/Core Data/CoreDataManagerTests.swift
Line: 699-722
Comment:
**`maxCount` capped by `fetchLimit` hides the real count**
In `countAudienceOccurrences`, `fetchRequest.fetchLimit` is set to `audienceOccurrence.maxCount` (which is `5` in this test), and `NSManagedObjectContext.count(for:)` respects `fetchLimit`. This means the test with `maxCount = 5` and 3 saved records will return `3` correctly, but if you had saved 6 records, the count would be capped at `5` (the fetchLimit), not `6`.
The test only validates the "below limit" case, so it doesn't exercise the cap behaviour. Consider adding a complementary case where `maxCount < numberOfSaves` to verify the limit is properly enforced (e.g., save 5 occurrences with `maxCount = 3` and assert `count == 3`).
How can I resolve this? If you propose a fix, please make it concise.
|
Closing — the root cause is that |
1 participant
Summary
countAudienceOccurrences()andcountPlacement()inCoreDataManagerwere delegating toCoreDataStack.count(), which runs queries on a separateNSManagedObjectContextfrom the oneCoreDataManageruses for saving occurrences. Under memory pressure (e.g. low-RAM devices with frequentprocessTerminatedevents), the cross-context merge notification can be delayed, causing the count query to return 0 and bypassing the configured audience interval limit.CoreDataManager's ownbackgroundContext— the same context used bysave(triggerAudienceOccurrence:)andsavePlacementData()— ensuring immediate visibility of saved records.KvFvRBoPVkMd6v93SqC2FpbbXzm1.Changes
Sources/SuperwallKit/Storage/Core Data/CoreDataManager.swift: RewrotecountAudienceOccurrences()andcountPlacement()to useself.backgroundContextdirectly instead ofcoreDataStack.count()Tests/SuperwallKitTests/Storage/Core Data/CoreDataManagerTests.swift: Added 3 tests validating save-then-count visibility on the same contextCHANGELOG.md: Added 4.14.3 entryChecklist
CHANGELOG.mdfor any breaking changes, enhancements, or bug fixes.swiftlintin the main directory and fixed any issues.🤖 Generated with Claude Code
Greptile Summary
This PR fixes a real production bug where audience occurrence interval limits were bypassed on low-RAM devices. The root cause was that
countAudienceOccurrences()andcountPlacement()were queryingCoreDataStack's privatebackgroundContextwhile all saves went throughCoreDataManager's separatebackgroundContext. Under memory pressure, the cross-context merge notification was delayed, causing the count to return 0 and allowing repeated audience matches within the configured interval.Key changes:
countAudienceOccurrences()andcountPlacement()now perform count queries directly onCoreDataManager.backgroundContext— the same context used by all saves — ensuring immediate read-your-own-writes consistency.Logger.debugis added for count failures, returning0gracefully.Issues found:
getComputedPropertySincePlacementstill delegates tocoreDataStack.getLastSavedPlacement(), which usesCoreDataStack's ownbackgroundContext. This is the same class of cross-context staleness bug that this PR is fixing, and it was not addressed here.XCTestCaseAPIs (XCTAssertEqual,expectation(description:),await fulfillment(of:)) instead of Swift's Testing framework, whichCLAUDE.mdmandates for all new tests.countPlacementfix, and no test covers thefetchLimitcap behaviour incountAudienceOccurrences.Confidence Score: 3/5
getComputedPropertySincePlacementretains the same cross-context staleness issue, and new tests violate the project's testing framework conventions.getComputedPropertySincePlacementis left using the old cross-context path (coreDataStack.getLastSavedPlacement), meaning the same class of bug persists for computed properties. The new tests also use XCTest instead of Swift Testing as required by CLAUDE.md, and test coverage forcountPlacement's same-context fix is missing. These gaps reduce confidence in completeness.Sources/SuperwallKit/Storage/Core Data/CoreDataManager.swift—getComputedPropertySincePlacementon line 168 still usescoreDataStackand needs the same treatment as the two fixed count methods.Important Files Changed
countAudienceOccurrencesandcountPlacementto useCoreDataManager's ownbackgroundContextdirectly instead ofcoreDataStack.count(), fixing a cross-context staleness bug.getComputedPropertySincePlacementstill delegates tocoreDataStackand is susceptible to the same issue.countAudienceOccurrenceson the same context, but uses XCTest APIs contrary to CLAUDE.md's mandate to use Swift Testing for new tests. No equivalent same-context tests were added forcountPlacement, and no test covers thefetchLimitcap behaviour.Sequence Diagram
sequenceDiagram participant Caller participant CoreDataManager participant CDM_bgCtx as CoreDataManager.backgroundContext participant CDStack_bgCtx as CoreDataStack.backgroundContext (old path) Note over Caller,CDStack_bgCtx: BEFORE (cross-context, stale under memory pressure) Caller->>CoreDataManager: save(triggerAudienceOccurrence:) CoreDataManager->>CDM_bgCtx: perform { save() } Note right of CDM_bgCtx: Record saved here Caller->>CoreDataManager: countAudienceOccurrences(for:) CoreDataManager->>CDStack_bgCtx: coreDataStack.count(fetchRequest) Note right of CDStack_bgCtx: Merge notification may be delayed! CDStack_bgCtx-->>CoreDataManager: returns 0 (stale) CoreDataManager-->>Caller: 0 ❌ (limit bypassed) Note over Caller,CDStack_bgCtx: AFTER (same context, always consistent) Caller->>CoreDataManager: save(triggerAudienceOccurrence:) CoreDataManager->>CDM_bgCtx: perform { save() } Note right of CDM_bgCtx: Record saved here Caller->>CoreDataManager: countAudienceOccurrences(for:) CoreDataManager->>CDM_bgCtx: perform { count(for: fetchRequest) } CDM_bgCtx-->>CoreDataManager: returns 1 (fresh) CoreDataManager-->>Caller: 1 ✅ (limit enforced)Comments Outside Diff (1)
Sources/SuperwallKit/Storage/Core Data/CoreDataManager.swift, line 177-196 ([link](https://github.com/superwall/superwall-ios/blob/7f6bdf9e3418a4e054f372033813b6ff829331ca/Sources/SuperwallKit/Storage/Core Data/CoreDataManager.swift#L177-L196))getComputedPropertySincePlacementdelegates tocoreDataStack.getLastSavedPlacement(), which queriesCoreDataStack's ownbackgroundContext— a completely separateNSManagedObjectContextfromCoreDataManager.backgroundContextthat is used for all saves. This is the exact same class of cross-context staleness bug that this PR fixes forcountAudienceOccurrencesandcountPlacement. Under memory pressure with delayed merge notifications, a freshly saved placement may not be visible to this query, causing computed property calculations to return stale results.Consider moving this logic to use
CoreDataManager's ownbackgroundContextdirectly, similar to the fix applied to the two count methods.Prompt To Fix With AI
Prompt To Fix All With AI
Last reviewed commit: "fix: use same Core D..."
Context used: