Skip to content

Commit

Permalink
Catch race condition in PostHogFileBackedQueue.deleteFiles (#218)
Browse files Browse the repository at this point in the history
* Catch race condition in PostHogFileBackedQueue.deleteFiles

In rare cases, `deleteFiles(_:)` can be invoked such that, between the check for items.isEmpty and the call to items.remove, the task gets pre-empted and items is mutated. Once the thread resumes, the `.remove(at: 0)` operation causes a crash.

* Update ReadWriteLock.

* Address feedback.
  • Loading branch information
jverkoey authored Oct 22, 2024
1 parent 1bb1709 commit 766933a
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 7 deletions.
12 changes: 8 additions & 4 deletions PostHog/PostHogFileBackedQueue.swift
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,14 @@ class PostHogFileBackedQueue {

private func deleteFiles(_ count: Int) {
for _ in 0 ..< count {
if items.isEmpty { return }
let removed = items.remove(at: 0) // We always remove from the top of the queue

deleteSafely(queue.appendingPathComponent(removed))
if let removed: String = _items.mutate({ items in
if items.isEmpty {
return nil
}
return items.remove(at: 0) // We always remove from the top of the queue
}) {
deleteSafely(queue.appendingPathComponent(removed))
}
}
}
}
9 changes: 6 additions & 3 deletions PostHog/Utils/ReadWriteLock.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,13 @@ public final class ReadWriteLock<Value> {
/// The lock will be acquired once for writing before invoking the closure.
///
/// - Parameter closure: The closure with the mutable value.
public func mutate(_ closure: (inout Value) -> Void) {
@discardableResult
public func mutate<T>(_ closure: (inout Value) -> T) -> T {
pthread_rwlock_wrlock(&rwlock)
closure(&value)
pthread_rwlock_unlock(&rwlock)
defer {
pthread_rwlock_unlock(&rwlock)
}
return closure(&value)
}
}

Expand Down

0 comments on commit 766933a

Please sign in to comment.