Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Commit

Permalink
Fix #2049: sharedSyncDevice race conditions. (#2050)
Browse files Browse the repository at this point in the history
Shared sync device was kept as a static variable. The thing is that, NSManagedObjects are not thread safe.

This changes the implementation so its thread safe object id is being kept and used to access the object.
  • Loading branch information
iccub authored and jhreis committed Dec 4, 2019
1 parent 838ead9 commit 63f1af7
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 8 deletions.
12 changes: 6 additions & 6 deletions Data/models/Device.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ private let log = Logger.browserLogger
public final class Device: NSManagedObject, Syncable, CRUD {

// Check if this can be nested inside the method
static var sharedCurrentDevice: Device?
static var sharedCurrentDeviceId: NSManagedObjectID?

// Assign on parent model via CD
@NSManaged public var isSynced: Bool
Expand Down Expand Up @@ -56,14 +56,14 @@ extension Device {

/// Returns a current device and assings it to a shared variable.
static func currentDevice(context: NSManagedObjectContext = DataController.viewContext) -> Device? {
if sharedCurrentDevice == nil {
guard let deviceId = sharedCurrentDeviceId else {
let predicate = NSPredicate(format: "isCurrentDevice == true")
sharedCurrentDevice = first(where: predicate, context: context)
} else if let sharedDevice = sharedCurrentDevice {
sharedCurrentDevice = context.object(with: sharedDevice.objectID) as? Device
let device = first(where: predicate, context: context)
sharedCurrentDeviceId = device?.objectID
return device
}

return sharedCurrentDevice
return context.object(with: deviceId) as? Device
}

class func add(name: String?, isCurrent: Bool = false) {
Expand Down
2 changes: 1 addition & 1 deletion Data/sync/Sync.swift
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ public class Sync: JSInjector {
self.sendSyncRecords(action: .delete, records: [device], context: context)
}

Device.sharedCurrentDevice = nil
Device.sharedCurrentDeviceId = nil
Device.deleteAll(context: .existing(context))
}

Expand Down
2 changes: 1 addition & 1 deletion DataTests/CoreDataTestCase.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class CoreDataTestCase: XCTestCase {
override func tearDown() {
NotificationCenter.default.removeObserver(self)
DataController.viewContext.reset()
Device.sharedCurrentDevice = nil
Device.sharedCurrentDeviceId = nil
contextSaveCompletion = nil
super.tearDown()
}
Expand Down

0 comments on commit 63f1af7

Please sign in to comment.