Skip to content

fix: audience occurrence interval limits bypassed under memory pressure #455

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
yusuftor wants to merge 1 commit into from
Closed

fix: audience occurrence interval limits bypassed under memory pressure #455

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

The changelog for `SuperwallKit`. Also see the [releases](https://github.com/superwall/Superwall-iOS/releases) on GitHub.

## 4.14.3

### Fixes

- Fixed audience occurrence interval limits being bypassed under memory pressure. The occurrence count query was running on a separate Core Data context from the one used to save occurrences, which could return stale results when cross-context propagation was delayed.

## 4.14.2

### Enhancements
Expand Down
64 changes: 46 additions & 18 deletions Sources/SuperwallKit/Storage/Core Data/CoreDataManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import Foundation
import CoreData

class CoreDataManager {

Check warning on line 11 in Sources/SuperwallKit/Storage/Core Data/CoreDataManager.swift

View workflow job for this annotation

GitHub Actions / Package-SwiftLint 

Type Body Length Violation: Class body should span 250 lines or less excluding comments and whitespace: currently spans 268 lines (type_body_length)
private let coreDataStack: CoreDataStack
private var backgroundContext: NSManagedObjectContext?

Expand Down Expand Up @@ -199,6 +199,10 @@
func countAudienceOccurrences(
for audienceOccurrence: TriggerAudienceOccurrence
) async -> Int {
guard let backgroundContext = backgroundContext else {
return 0
}

let fetchRequest = ManagedTriggerRuleOccurrence.fetchRequest()
fetchRequest.fetchLimit = audienceOccurrence.maxCount

Expand All @@ -222,17 +226,27 @@
date as NSDate,
audienceOccurrence.key
)

return await withCheckedContinuation { continuation in
coreDataStack.count(for: fetchRequest) { count in
continuation.resume(returning: count)
}
}
case .infinity:
fetchRequest.predicate = NSPredicate(format: "occurrenceKey == %@", audienceOccurrence.key)
return await withCheckedContinuation { continuation in
coreDataStack.count(for: fetchRequest) { count in
fetchRequest.predicate = NSPredicate(
format: "occurrenceKey == %@",
audienceOccurrence.key
)
}

return await withCheckedContinuation { continuation in
backgroundContext.perform {
do {
let count = try backgroundContext.count(for: fetchRequest)
continuation.resume(returning: count)
} catch let error as NSError {
Logger.debug(
logLevel: .error,
scope: .coreData,
message: "Error counting audience occurrences from Core Data.",
info: error.userInfo,
error: error
)
continuation.resume(returning: 0)
}
}
}
Expand All @@ -242,6 +256,10 @@
_ placementName: String,
interval: TriggerAudienceOccurrence.Interval
) async -> Int {
guard let backgroundContext = backgroundContext else {
return 0
}

let fetchRequest = ManagedEventData.fetchRequest()

switch interval {
Expand All @@ -264,17 +282,27 @@
date as NSDate,
placementName
)

return await withCheckedContinuation { continuation in
coreDataStack.count(for: fetchRequest) { count in
continuation.resume(returning: count)
}
}
case .infinity:
fetchRequest.predicate = NSPredicate(format: "name == %@", placementName)
return await withCheckedContinuation { continuation in
coreDataStack.count(for: fetchRequest) { count in
fetchRequest.predicate = NSPredicate(
format: "name == %@",
placementName
)
}

return await withCheckedContinuation { continuation in
backgroundContext.perform {
do {
let count = try backgroundContext.count(for: fetchRequest)
continuation.resume(returning: count)
} catch let error as NSError {
Logger.debug(
logLevel: .error,
scope: .coreData,
message: "Error counting placements from Core Data.",
info: error.userInfo,
error: error
)
continuation.resume(returning: 0)
}
}
}
Expand Down
71 changes: 71 additions & 0 deletions Tests/SuperwallKitTests/Storage/Core Data/CoreDataManagerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,77 @@ class CoreDataManagerTests: XCTestCase {
XCTAssertEqual(userEventCount, 0, "paywall_decline should be deleted after deleteAllEntities")
}

// MARK: - Save and Count on Same Context

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")
}
Comment on lines +655 to +722
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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)

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!

Comment on lines +699 to +722
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.


func test_countPlacement_concurrent_access() async {

let placementName = "concurrent_test"
Expand Down
Loading