Skip to content

Commit

Permalink
Merge pull request #222 from trupin/master
Browse files Browse the repository at this point in the history 
Fixed race conditions in dispose bag and subjects with unit testing
  • Loading branch information
@srdanrasic
srdanrasic authored Jun 21, 2019
2 parents 4bc3a10 + 21b94cf commit 3d82fba
Show file tree
Hide file tree
Showing 5 changed files with 306 additions and 11 deletions.
4 changes: 4 additions & 0 deletions ReactiveKit.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
16C33B841BEFBAC900A0DBE0 /* ReactiveKit.h in Headers */ = {isa = PBXBuildFile; fileRef = ECBCCDD31BEB6B9A00723476 /* ReactiveKit.h */; settings = {ATTRIBUTES = (Public, ); }; };
16C33B851BEFBAC900A0DBE0 /* ReactiveKit.h in Headers */ = {isa = PBXBuildFile; fileRef = ECBCCDD31BEB6B9A00723476 /* ReactiveKit.h */; settings = {ATTRIBUTES = (Public, ); }; };
16D30EBD1D6595AB00C2435D /* ReactiveKit.h in Headers */ = {isa = PBXBuildFile; fileRef = ECBCCDD31BEB6B9A00723476 /* ReactiveKit.h */; settings = {ATTRIBUTES = (Public, ); }; };
19276399228F779200EDF2C0 /* SubjectTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 19276398228F779200EDF2C0 /* SubjectTests.swift */; };
EC48D30D224FB5C400284EA0 /* SignalProtocol+Utilities.swift in Sources */ = {isa = PBXBuildFile; fileRef = EC48D2F1224FB5C400284EA0 /* SignalProtocol+Utilities.swift */; };
EC48D30E224FB5C400284EA0 /* SignalProtocol+Utilities.swift in Sources */ = {isa = PBXBuildFile; fileRef = EC48D2F1224FB5C400284EA0 /* SignalProtocol+Utilities.swift */; };
EC48D30F224FB5C400284EA0 /* SignalProtocol+Utilities.swift in Sources */ = {isa = PBXBuildFile; fileRef = EC48D2F1224FB5C400284EA0 /* SignalProtocol+Utilities.swift */; };
Expand Down Expand Up @@ -146,6 +147,7 @@
16C33AF91BEFB72500A0DBE0 /* ReactiveKit.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = ReactiveKit.framework; sourceTree = BUILT_PRODUCTS_DIR; };
16C33B161BEFB9CB00A0DBE0 /* ReactiveKit.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = ReactiveKit.framework; sourceTree = BUILT_PRODUCTS_DIR; };
16C33B241BEFBA0100A0DBE0 /* ReactiveKit.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = ReactiveKit.framework; sourceTree = BUILT_PRODUCTS_DIR; };
19276398228F779200EDF2C0 /* SubjectTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SubjectTests.swift; sourceTree = "<group>"; };
EC48D2F1224FB5C400284EA0 /* SignalProtocol+Utilities.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "SignalProtocol+Utilities.swift"; sourceTree = "<group>"; };
EC48D2F2224FB5C400284EA0 /* Observer.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Observer.swift; sourceTree = "<group>"; };
EC48D2F3224FB5C400284EA0 /* SignalProtocol+Monad.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "SignalProtocol+Monad.swift"; sourceTree = "<group>"; };
Expand Down Expand Up @@ -275,6 +277,7 @@
EC48D381224FB66100284EA0 /* Helpers.swift */,
EC48D382224FB66100284EA0 /* SignalTests.swift */,
EC48D380224FB66100284EA0 /* PropertyTests.swift */,
19276398228F779200EDF2C0 /* SubjectTests.swift */,
);
path = ReactiveKitTests;
sourceTree = "<group>";
Expand Down Expand Up @@ -681,6 +684,7 @@
isa = PBXSourcesBuildPhase;
buildActionMask = 2147483647;
files = (
19276399228F779200EDF2C0 /* SubjectTests.swift in Sources */,
EC48D384224FB66100284EA0 /* PropertyTests.swift in Sources */,
EC48D386224FB66100284EA0 /* SignalTests.swift in Sources */,
EC48D385224FB66100284EA0 /* Helpers.swift in Sources */,
Expand Down
13 changes: 9 additions & 4 deletions Sources/Disposable.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,10 @@ public final class DisposeBag: DisposeBagProtocol {

private var disposables: [Disposable] = []
private var subject: ReplayOneSubject<Void, Never>?
private lazy var lock = NSRecursiveLock(name: "com.reactivekit.disposebag")


private let subjectLoadingLock = NSRecursiveLock(name: "com.reactivekit.disposebag.subject")
private let disposablesLock = NSRecursiveLock(name: "com.reactivekit.disposebag.disposables")

/// `true` if bag is empty, `false` otherwise.
public var isDisposed: Bool {
return disposables.count == 0
Expand All @@ -222,12 +224,14 @@ public final class DisposeBag: DisposeBagProtocol {
/// Add the given disposable to the bag.
/// Disposable will be disposed when the bag is deallocated.
public func add(disposable: Disposable) {
disposablesLock.lock(); defer { disposablesLock.unlock() }
disposables.append(disposable)
}

/// Add the given disposables to the bag.
/// Disposables will be disposed when the bag is deallocated.
public func add(disposables: [Disposable]) {
disposablesLock.lock(); defer { disposablesLock.unlock() }
disposables.forEach(add)
}

Expand All @@ -243,17 +247,18 @@ public final class DisposeBag: DisposeBagProtocol {

/// Disposes all disposables that are currenty in the bag.
public func dispose() {
disposablesLock.lock(); defer { disposablesLock.unlock() }
disposables.forEach { $0.dispose() }
disposables.removeAll()
}

/// A signal that fires `completed` event when the bag gets deallocated.
public var deallocated: SafeSignal<Void> {
lock.lock()
subjectLoadingLock.lock()
if subject == nil {
subject = ReplayOneSubject()
}
lock.unlock()
subjectLoadingLock.unlock()
return subject!.toSignal()
}

Expand Down
5 changes: 3 additions & 2 deletions Sources/Property.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public protocol PropertyProtocol {
public final class Property<Value>: PropertyProtocol, SubjectProtocol, BindableProtocol, DisposeBagProvider {

private var _value: Value
private let subject = PublishSubject<Value, Never>()
private let subject: Subject<Value, Never>
private let lock = NSRecursiveLock(name: "reactive_kit.property")

public var bag: DisposeBag {
Expand All @@ -54,8 +54,9 @@ public final class Property<Value>: PropertyProtocol, SubjectProtocol, BindableP
}
}

public init(_ value: Value) {
public init(_ value: Value, subject: Subject<Value, Never> = PublishSubject()) {
_value = value
self.subject = subject
}

public func on(_ event: Event<Value, Never>) {
Expand Down
13 changes: 8 additions & 5 deletions Sources/Subjects.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,26 +38,26 @@ open class Subject<Element, Error: Swift.Error>: SubjectProtocol {

public private(set) var isTerminated = false

public let observersLock = NSRecursiveLock(name: "reactive_kit.subject.observers_lock")
public let dispatchLock = NSRecursiveLock(name: "reactive_kit.subject.dispatch_lock")
public let lock = NSRecursiveLock(name: "reactive_kit.subject.lock")

public let disposeBag = DisposeBag()

public init() {}

public func on(_ event: Event<Element, Error>) {
dispatchLock.lock(); defer { dispatchLock.unlock() }
lock.lock(); defer { lock.unlock() }
guard !isTerminated else { return }
isTerminated = event.isTerminal
send(event)
}

open func send(_ event: Event<Element, Error>) {
lock.lock(); defer { lock.unlock() }
forEachObserver { $0(event) }
}

open func observe(with observer: @escaping Observer<Element, Error>) -> Disposable {
observersLock.lock(); defer { observersLock.unlock() }
lock.lock(); defer { lock.unlock() }
willAdd(observer: observer)
return add(observer: observer)
}
Expand All @@ -73,7 +73,7 @@ open class Subject<Element, Error: Swift.Error>: SubjectProtocol {

return BlockDisposable { [weak self] in
guard let me = self else { return }
me.observersLock.lock(); defer { me.observersLock.unlock() }
me.lock.lock(); defer { me.lock.unlock() }
guard let index = me.observers.firstIndex(where: { $0.0 == token }) else { return }
me.observers.remove(at: index)
}
Expand Down Expand Up @@ -120,6 +120,7 @@ public final class ReplaySubject<Element, Error: Swift.Error>: Subject<Element,
}

public override func send(_ event: Event<Element, Error>) {
lock.lock(); defer { lock.unlock() }
buffer.append(event)
buffer = buffer.suffix(bufferSize)
super.send(event)
Expand All @@ -140,6 +141,7 @@ public final class ReplayOneSubject<Element, Error: Swift.Error>: Subject<Elemen
private var terminalEvent: Event<Element, Error>? = nil

public override func send(_ event: Event<Element, Error>) {
lock.lock(); defer { lock.unlock() }
if event.isTerminal {
terminalEvent = event
} else {
Expand Down Expand Up @@ -181,6 +183,7 @@ public final class ReplayLoadingValueSubject<Val, LoadingError: Swift.Error, Err
}

public override func send(_ event: Event<LoadingState<Val, LoadingError>, Error>) {
lock.lock(); defer { lock.unlock() }
switch event {
case .next(let loadingState):
switch loadingState {
Expand Down
Loading

0 comments on commit 3d82fba

Please sign in to comment.