Skip to content
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.

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

[iOS] Fix data race in PersistentFileLogSpec.swift #28924

Merged
1 change: 1 addition & 0 deletions packages/expo-modules-core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
- [Android] Fix `getContext().getNativeModule(UIManagerModule.class)` in Bridgeless. ([#29203](https://github.com/expo/expo/pull/29203) by [@arushikesarwani94](https://github.com/arushikesarwani94))
- [Android] Fixed converting from `null` to `Record` sometimes didn't work as expected. ([#29508](https://github.com/expo/expo/pull/29508) by [@lukmccall](https://github.com/lukmccall))
- [Android] Fixed `No implementation found for com.facebook.jni.HybridData expo.modules.kotlin.jni.JavaScriptModuleObject.initHybrid`. ([#29513](https://github.com/expo/expo/pull/29513) by [@lukmccall](https://github.com/lukmccall))
- [iOS] Fix data race in `PersistentFileLogSpec.swift`. ([#28924](https://github.com/expo/expo/pull/28924) by [@hakonk](https://github.com/hakonk))

### 💡 Others

Expand Down
49 changes: 40 additions & 9 deletions packages/expo-modules-core/ios/Tests/PersistentFileLogSpec.swift
Original file line number Diff line number Diff line change
Expand Up @@ -48,26 +48,57 @@ class PersistentFileLogSpec: ExpoSpec {
}

static func clearEntriesSync(log: PersistentFileLog) {
var didClear = false
let didClear = Synchronized(false)
log.clearEntries { _ in
didClear = true
didClear.value = true
}
expect(didClear).toEventually(beTrue(), timeout: .milliseconds(500))
expect(didClear.value).toEventually(beTrue(), timeout: .milliseconds(500))
}

static func filterEntriesSync(log: PersistentFileLog, filter: @escaping PersistentFileLogFilter) {
var didPurge = false
let didPurge = Synchronized(false)
log.purgeEntriesNotMatchingFilter(filter: filter) { _ in
didPurge = true
didPurge.value = true
}
expect(didPurge).toEventually(beTrue(), timeout: .milliseconds(500))
expect(didPurge.value).toEventually(beTrue(), timeout: .milliseconds(500))
}

static func appendEntrySync(log: PersistentFileLog, entry: String) {
var didAppend = false
let didAppend = Synchronized(false)
log.appendEntry(entry: entry) { _ in
didAppend = true
didAppend.value = true
}
expect(didAppend).toEventually(beTrue(), timeout: .milliseconds(500))
expect(didAppend.value).toEventually(beTrue(), timeout: .milliseconds(500))
}
}

/// Allows for synchronization pertaining to the file scope.
private final class Synchronized<T> {
private var _storage: T
private let lock = NSLock()

/// Thread safe access here.
var value: T {
get {
return lockAround {
_storage
}
}
set {
lockAround {
_storage = newValue
}
}
}

init(_ storage: T) {
self._storage = storage
}

private func lockAround<U>(_ closure: () -> U) -> U {
lock.lock()
defer { lock.unlock() }
return closure()
}

}
2 changes: 2 additions & 0 deletions packages/expo-updates/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

### 🐛 Bug fixes

- Fix data race in `AppLauncherWithDatabaseMock.swift`. ([#28924](https://github.com/expo/expo/pull/28924) by [@hakonk](https://github.com/hakonk))

### 💡 Others

- Removed redundant usage of `EventEmitter` instance. ([#28946](https://github.com/expo/expo/pull/28946) by [@tsapeta](https://github.com/tsapeta))
Expand Down
42 changes: 34 additions & 8 deletions packages/expo-updates/ios/Tests/AppLauncherWithDatabaseSpec.swift
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,11 @@ class AppLauncherWithDatabaseSpec : ExpoSpec {
directory: testDatabaseDir,
completionQueue: DispatchQueue.global(qos: .default)
)
var successValue: Bool? = nil
let successValue = Synchronized<Bool?>(nil)
launcher.launchUpdate(withSelectionPolicy: SelectionPolicyFactory.filterAwarePolicy(withRuntimeVersion: "1")) { error, success in
successValue = success
successValue.value = success
}

while successValue == nil {
Thread.sleep(forTimeInterval: 0.1)
}

expect(successValue) == true
expect(successValue.value).toEventually(beTrue(), timeout: .milliseconds(10_000))

db.databaseQueue.sync {
let sameUpdate = try! db.update(withId: testUpdate.updateId, config: config)
Expand All @@ -114,3 +109,34 @@ class AppLauncherWithDatabaseSpec : ExpoSpec {
}
}
}

/// Allows for synchronization pertaining to the file scope.
private final class Synchronized<T> {
private var _storage: T
private let lock = NSLock()

/// Thread safe access here.
var value: T {
get {
return lockAround {
_storage
}
}
set {
lockAround {
_storage = newValue
}
}
}

init(_ storage: T) {
self._storage = storage
}

private func lockAround<U>(_ closure: () -> U) -> U {
lock.lock()
defer { lock.unlock() }
return closure()
}

}
Loading