Skip to content

Commit 69e9f20

Browse files
LK-SimonLK-Simon
committed
1.0.4 - Fixes for issues with Thread-Safe Locking
Looking to resolve an issue with the Thread-Safe Locking of `ObservableThread` and `ObservableThreadSafeClass`.
1 parent 95ff51d commit 69e9f20

File tree

3 files changed

+55
-45
lines changed

3 files changed

+55
-45
lines changed

‎README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ let package = Package(
2727
dependencies: [
2828
.package(
2929
url: "https://github.com/Flowduino/Observable.git",
30-
.upToNextMajor(from: "1.0.1")
30+
.upToNextMajor(from: "1.0.4")
3131
),
3232
],
3333
//...

‎Sources/Observable/ObservableThread.swift

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import ThreadSafeSwift
1212
/**
1313
Provides custom Observer subscription and notification behaviour for Threads
1414
- Author: Simon J. Stuart
15-
- Version: 1.0
15+
- Version: 1.0.4
1616
- Note: The Observers are behind a Semaphore Lock
1717
- Note: A "Revolving Door" solution has been implemented to ensure that Observer Callbacks can modify the Observers (add/remove) without causing a Deadlock.
1818
*/
@@ -44,47 +44,52 @@ open class ObservableThread: Thread, Observable, ObservableObject {
4444
/**
4545
Dictionary mapping an `ObjectIdentifer` (reference to an Observer Instance) against its `ObserverContainer`
4646
- Author: Simon J. Stuart
47-
- Version: 1.0
47+
- Version: 1.0.4
4848
*/
49-
@ThreadSafeSemaphore private var observers = [ObjectIdentifier : ObserverContainer]()
50-
49+
private var observers = [ObjectIdentifier : ObserverContainer]()
50+
private var observerLock = DispatchSemaphore(value: 1)
5151

5252
/**
5353
Dictionary mapping an `ObjectIdentifer` (reference to an Observer Instance) against its `ObserverContainer`
5454
- Author: Simon J. Stuart
55-
- Version: 1.0
55+
- Version: 1.0.4
5656
- Note: This is used as a temporary "Holding Queue" when the `observers` Dictionary has its Lock retained by another Thread.
5757
*/
58-
@ThreadSafeSemaphore private var observersAddQueue = [ObjectIdentifier : ObserverContainer]()
58+
private var observersAddQueue = [ObjectIdentifier : ObserverContainer]()
59+
private var observerAddLock = DispatchSemaphore(value: 1)
60+
5961
/**
6062
Dictionary mapping an `ObjectIdentifer` (reference to an Observer Instance) against its `ObserverContainer`
6163
- Author: Simon J. Stuart
62-
- Version: 1.0
64+
- Version: 1.0.4
6365
- Note: This is used as a temporary "Holding Queue" when the `observers` Dictionary has its Lock retained by another Thread.
6466
*/
65-
@ThreadSafeSemaphore private var observersRemoveQueue = [ObjectIdentifier : ObserverContainer]()
67+
private var observersRemoveQueue = [ObjectIdentifier : ObserverContainer]()
68+
private var observerRemoveLock = DispatchSemaphore(value: 1)
6669

6770
public func addObserver<TObservationProtocol: AnyObject>(_ observer: TObservationProtocol) {
68-
var collection = _observers.lock.wait(timeout: DispatchTime.now()) == .success ? _observers : _observersAddQueue
69-
collection.wrappedValue[ObjectIdentifier(observer)] = ObserverContainer(observer: observer, dispatchQueue: OperationQueue.current?.underlyingQueue)
70-
collection.lock.signal()
71+
let lock = observerLock.wait(timeout: DispatchTime.now().advanced(by: DispatchTimeInterval.milliseconds(10))) == .success ? observerLock : observerAddLock
72+
var collection = ObjectIdentifier(lock) == ObjectIdentifier(observerLock) ? observers : observersAddQueue
73+
collection[ObjectIdentifier(observer)] = ObserverContainer(observer: observer, dispatchQueue: OperationQueue.current?.underlyingQueue)
74+
lock.signal()
7175
}
7276

7377
public func removeObserver<TObservationProtocol: AnyObject>(_ observer: TObservationProtocol) {
74-
let lockResult = _observers.lock.wait(timeout: DispatchTime.now())
78+
let lockResult = observerLock.wait(timeout: DispatchTime.now().advanced(by: DispatchTimeInterval.milliseconds(10)))
79+
7580
if lockResult == .success { // If we can obtain the lock for the Main Observer Collection...
7681
observers.removeValue(forKey: ObjectIdentifier(observer)) // Simply remove it from the Collection
77-
_observers.lock.signal() // Release the Lock
82+
observerLock.signal() // Release the Lock
7883
}
7984
else { // If we CAN'T get the Main Observer Collection's Lock...
80-
_observersRemoveQueue.lock.wait() // Get the Remove Queue Lock
85+
observerRemoveLock.wait() // Get the Remove Queue Lock
8186
observersRemoveQueue[ObjectIdentifier(observer)] = ObserverContainer(observer: observer, dispatchQueue: nil) // the Dispatch Queue doesn't matter here!
82-
_observersRemoveQueue.lock.signal() // Release the Remove Queue Lock
87+
observerRemoveLock.signal() // Release the Remove Queue Lock
8388
}
8489
}
8590

8691
public func withObservers<TObservationProtocol>(_ code: @escaping (_ observer: TObservationProtocol) -> ()) {
87-
self._observers.lock.wait()
92+
self.observerLock.wait()
8893
for (id, observation) in observers {
8994
guard let observer = observation.observer else { // Check if the Observer still exists
9095
observers.removeValue(forKey: id) // If it doesn't, remove the Observer from the collection...
@@ -99,17 +104,17 @@ open class ObservableThread: Thread, Observable, ObservableObject {
99104
}
100105
}
101106

102-
self._observersAddQueue.lock.wait() // Lock the Add Queue
103-
self._observersRemoveQueue.lock.wait() // Lock the Remove Queue
107+
self.observerAddLock.wait() // Lock the Add Queue
108+
self.observerRemoveLock.wait() // Lock the Remove Queue
104109
for (id, observation) in observersAddQueue { // Add all of the Queued Observers
105110
observers[id] = observation
106111
}
107112
for (id) in observersRemoveQueue.keys { // Remove all of the Queued Observers
108113
observers.removeValue(forKey: id)
109114
}
110-
self._observersAddQueue.lock.signal() // Release the Add Queue Lock
111-
self._observersRemoveQueue.lock.signal() // Release the Remove Queue Lock
112-
self._observers.lock.signal()
115+
self.observerAddLock.signal() // Release the Add Queue Lock
116+
self.observerRemoveLock.signal() // Release the Remove Queue Lock
117+
self.observerLock.signal()
113118
}
114119

115120
open func notifyChange() {

‎Sources/Observable/ObservableThreadSafeClass.swift

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ import Foundation
1010
import ThreadSafeSwift
1111

1212
/**
13-
Provides custom Observer subscription and notification behaviour for Threads
13+
Provides custom Observer subscription and notification behaviour for Classes that will be interacting with Multiple Threads
1414
- Author: Simon J. Stuart
15-
- Version: 1.0
15+
- Version: 1.0.4
1616
- Note: The Observers are behind a Semaphore Lock
1717
- Note: A "Revolving Door" solution has been implemented to ensure that Observer Callbacks can modify the Observers (add/remove) without causing a Deadlock.
1818
*/
@@ -44,47 +44,52 @@ open class ObservableThreadSafeClass: Observable, ObservableObject {
4444
/**
4545
Dictionary mapping an `ObjectIdentifer` (reference to an Observer Instance) against its `ObserverContainer`
4646
- Author: Simon J. Stuart
47-
- Version: 1.0
47+
- Version: 1.0.4
4848
*/
49-
@ThreadSafeSemaphore private var observers = [ObjectIdentifier : ObserverContainer]()
50-
49+
private var observers = [ObjectIdentifier : ObserverContainer]()
50+
private var observerLock = DispatchSemaphore(value: 1)
5151

5252
/**
5353
Dictionary mapping an `ObjectIdentifer` (reference to an Observer Instance) against its `ObserverContainer`
5454
- Author: Simon J. Stuart
55-
- Version: 1.0
55+
- Version: 1.0.4
5656
- Note: This is used as a temporary "Holding Queue" when the `observers` Dictionary has its Lock retained by another Thread.
5757
*/
58-
@ThreadSafeSemaphore private var observersAddQueue = [ObjectIdentifier : ObserverContainer]()
58+
private var observersAddQueue = [ObjectIdentifier : ObserverContainer]()
59+
private var observerAddLock = DispatchSemaphore(value: 1)
60+
5961
/**
6062
Dictionary mapping an `ObjectIdentifer` (reference to an Observer Instance) against its `ObserverContainer`
6163
- Author: Simon J. Stuart
62-
- Version: 1.0
64+
- Version: 1.0.4
6365
- Note: This is used as a temporary "Holding Queue" when the `observers` Dictionary has its Lock retained by another Thread.
6466
*/
65-
@ThreadSafeSemaphore private var observersRemoveQueue = [ObjectIdentifier : ObserverContainer]()
67+
private var observersRemoveQueue = [ObjectIdentifier : ObserverContainer]()
68+
private var observerRemoveLock = DispatchSemaphore(value: 1)
6669

6770
public func addObserver<TObservationProtocol: AnyObject>(_ observer: TObservationProtocol) {
68-
var collection = _observers.lock.wait(timeout: DispatchTime.now()) == .success ? _observers : _observersAddQueue
69-
collection.wrappedValue[ObjectIdentifier(observer)] = ObserverContainer(observer: observer, dispatchQueue: OperationQueue.current?.underlyingQueue)
70-
collection.lock.signal()
71+
let lock = observerLock.wait(timeout: DispatchTime.now().advanced(by: DispatchTimeInterval.milliseconds(10))) == .success ? observerLock : observerAddLock
72+
var collection = ObjectIdentifier(lock) == ObjectIdentifier(observerLock) ? observers : observersAddQueue
73+
collection[ObjectIdentifier(observer)] = ObserverContainer(observer: observer, dispatchQueue: OperationQueue.current?.underlyingQueue)
74+
lock.signal()
7175
}
7276

7377
public func removeObserver<TObservationProtocol: AnyObject>(_ observer: TObservationProtocol) {
74-
let lockResult = _observers.lock.wait(timeout: DispatchTime.now())
78+
let lockResult = observerLock.wait(timeout: DispatchTime.now().advanced(by: DispatchTimeInterval.milliseconds(10)))
79+
7580
if lockResult == .success { // If we can obtain the lock for the Main Observer Collection...
7681
observers.removeValue(forKey: ObjectIdentifier(observer)) // Simply remove it from the Collection
77-
_observers.lock.signal() // Release the Lock
82+
observerLock.signal() // Release the Lock
7883
}
7984
else { // If we CAN'T get the Main Observer Collection's Lock...
80-
_observersRemoveQueue.lock.wait() // Get the Remove Queue Lock
85+
observerRemoveLock.wait() // Get the Remove Queue Lock
8186
observersRemoveQueue[ObjectIdentifier(observer)] = ObserverContainer(observer: observer, dispatchQueue: nil) // the Dispatch Queue doesn't matter here!
82-
_observersRemoveQueue.lock.signal() // Release the Remove Queue Lock
87+
observerRemoveLock.signal() // Release the Remove Queue Lock
8388
}
8489
}
8590

8691
public func withObservers<TObservationProtocol>(_ code: @escaping (_ observer: TObservationProtocol) -> ()) {
87-
self._observers.lock.wait()
92+
self.observerLock.wait()
8893
for (id, observation) in observers {
8994
guard let observer = observation.observer else { // Check if the Observer still exists
9095
observers.removeValue(forKey: id) // If it doesn't, remove the Observer from the collection...
@@ -99,17 +104,17 @@ open class ObservableThreadSafeClass: Observable, ObservableObject {
99104
}
100105
}
101106

102-
self._observersAddQueue.lock.wait() // Lock the Add Queue
103-
self._observersRemoveQueue.lock.wait() // Lock the Remove Queue
107+
self.observerAddLock.wait() // Lock the Add Queue
108+
self.observerRemoveLock.wait() // Lock the Remove Queue
104109
for (id, observation) in observersAddQueue { // Add all of the Queued Observers
105110
observers[id] = observation
106111
}
107112
for (id) in observersRemoveQueue.keys { // Remove all of the Queued Observers
108113
observers.removeValue(forKey: id)
109114
}
110-
self._observersAddQueue.lock.signal() // Release the Add Queue Lock
111-
self._observersRemoveQueue.lock.signal() // Release the Remove Queue Lock
112-
self._observers.lock.signal()
115+
self.observerAddLock.signal() // Release the Add Queue Lock
116+
self.observerRemoveLock.signal() // Release the Remove Queue Lock
117+
self.observerLock.signal()
113118
}
114119

115120
open func notifyChange() {
@@ -125,6 +130,6 @@ open class ObservableThreadSafeClass: Observable, ObservableObject {
125130
}
126131

127132
public init() {
128-
133+
129134
}
130135
}

0 commit comments

Comments
 (0)