diff --git a/SSignalKit.xcodeproj/project.pbxproj b/SSignalKit.xcodeproj/project.pbxproj index 3315b248d5..0633d6faa2 100644 --- a/SSignalKit.xcodeproj/project.pbxproj +++ b/SSignalKit.xcodeproj/project.pbxproj @@ -29,8 +29,6 @@ D0445E351A7C2D7300267924 /* SSignal+Catch.m in Sources */ = {isa = PBXBuildFile; fileRef = D0445E091A7C2D7300267924 /* SSignal+Catch.m */; }; D0445E361A7C2D7300267924 /* SSignal+Combine.h in Headers */ = {isa = PBXBuildFile; fileRef = D0445E0A1A7C2D7300267924 /* SSignal+Combine.h */; settings = {ATTRIBUTES = (Public, ); }; }; D0445E371A7C2D7300267924 /* SSignal+Combine.m in Sources */ = {isa = PBXBuildFile; fileRef = D0445E0B1A7C2D7300267924 /* SSignal+Combine.m */; }; - D0445E381A7C2D7300267924 /* SSignal+Concat.h in Headers */ = {isa = PBXBuildFile; fileRef = D0445E0C1A7C2D7300267924 /* SSignal+Concat.h */; settings = {ATTRIBUTES = (Public, ); }; }; - D0445E391A7C2D7300267924 /* SSignal+Concat.m in Sources */ = {isa = PBXBuildFile; fileRef = D0445E0D1A7C2D7300267924 /* SSignal+Concat.m */; }; D0445E3A1A7C2D7300267924 /* SSignal+Dispatch.h in Headers */ = {isa = PBXBuildFile; fileRef = D0445E0E1A7C2D7300267924 /* SSignal+Dispatch.h */; settings = {ATTRIBUTES = (Public, ); }; }; D0445E3B1A7C2D7300267924 /* SSignal+Dispatch.m in Sources */ = {isa = PBXBuildFile; fileRef = D0445E0F1A7C2D7300267924 /* SSignal+Dispatch.m */; }; D0445E3C1A7C2D7300267924 /* SSignal+Mapping.h in Headers */ = {isa = PBXBuildFile; fileRef = D0445E101A7C2D7300267924 /* SSignal+Mapping.h */; settings = {ATTRIBUTES = (Public, ); }; }; @@ -65,7 +63,6 @@ D0445E7B1A7C447D00267924 /* SSignal+Accumulate.m in Sources */ = {isa = PBXBuildFile; fileRef = D0445E071A7C2D7300267924 /* SSignal+Accumulate.m */; }; D0445E7C1A7C447D00267924 /* SSignal+Catch.m in Sources */ = {isa = PBXBuildFile; fileRef = D0445E091A7C2D7300267924 /* SSignal+Catch.m */; }; D0445E7D1A7C447D00267924 /* SSignal+Combine.m in Sources */ = {isa = PBXBuildFile; fileRef = D0445E0B1A7C2D7300267924 /* SSignal+Combine.m */; }; - D0445E7E1A7C447D00267924 /* SSignal+Concat.m in Sources */ = {isa = PBXBuildFile; fileRef = D0445E0D1A7C2D7300267924 /* SSignal+Concat.m */; }; D0445E7F1A7C447D00267924 /* SSignal+Dispatch.m in Sources */ = {isa = PBXBuildFile; fileRef = D0445E0F1A7C2D7300267924 /* SSignal+Dispatch.m */; }; D0445E801A7C447D00267924 /* SSignal+Mapping.m in Sources */ = {isa = PBXBuildFile; fileRef = D0445E111A7C2D7300267924 /* SSignal+Mapping.m */; }; D0445E811A7C447D00267924 /* SSignal+Meta.m in Sources */ = {isa = PBXBuildFile; fileRef = D0445E131A7C2D7300267924 /* SSignal+Meta.m */; }; @@ -133,8 +130,6 @@ D0445E091A7C2D7300267924 /* SSignal+Catch.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = "SSignal+Catch.m"; sourceTree = ""; }; D0445E0A1A7C2D7300267924 /* SSignal+Combine.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "SSignal+Combine.h"; sourceTree = ""; }; D0445E0B1A7C2D7300267924 /* SSignal+Combine.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = "SSignal+Combine.m"; sourceTree = ""; }; - D0445E0C1A7C2D7300267924 /* SSignal+Concat.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "SSignal+Concat.h"; sourceTree = ""; }; - D0445E0D1A7C2D7300267924 /* SSignal+Concat.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = "SSignal+Concat.m"; sourceTree = ""; }; D0445E0E1A7C2D7300267924 /* SSignal+Dispatch.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "SSignal+Dispatch.h"; sourceTree = ""; }; D0445E0F1A7C2D7300267924 /* SSignal+Dispatch.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = "SSignal+Dispatch.m"; sourceTree = ""; }; D0445E101A7C2D7300267924 /* SSignal+Mapping.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "SSignal+Mapping.h"; sourceTree = ""; }; @@ -239,8 +234,6 @@ D0445E091A7C2D7300267924 /* SSignal+Catch.m */, D0445E0A1A7C2D7300267924 /* SSignal+Combine.h */, D0445E0B1A7C2D7300267924 /* SSignal+Combine.m */, - D0445E0C1A7C2D7300267924 /* SSignal+Concat.h */, - D0445E0D1A7C2D7300267924 /* SSignal+Concat.m */, D0445E0E1A7C2D7300267924 /* SSignal+Dispatch.h */, D0445E0F1A7C2D7300267924 /* SSignal+Dispatch.m */, D0445E101A7C2D7300267924 /* SSignal+Mapping.h */, @@ -327,7 +320,6 @@ D0445E231A7C2D7300267924 /* SAtomic.h in Headers */, D087632A1A839EDC00632240 /* SDisposableSet.h in Headers */, D0445E461A7C2D7300267924 /* SSignal+Timing.h in Headers */, - D0445E381A7C2D7300267924 /* SSignal+Concat.h in Headers */, D0445DDE1A7C2CA500267924 /* SSignalKit.h in Headers */, D0445E3C1A7C2D7300267924 /* SSignal+Mapping.h in Headers */, ); @@ -467,7 +459,6 @@ D0445E311A7C2D7300267924 /* SSignal.m in Sources */, D087632B1A839EDC00632240 /* SDisposableSet.m in Sources */, D0445E491A7C2D7300267924 /* SSubscriber.m in Sources */, - D0445E391A7C2D7300267924 /* SSignal+Concat.m in Sources */, D0445E431A7C2D7300267924 /* SSignal+SideEffects.m in Sources */, D0445E3F1A7C2D7300267924 /* SSignal+Meta.m in Sources */, D0445E371A7C2D7300267924 /* SSignal+Combine.m in Sources */, @@ -508,7 +499,6 @@ D0445E7A1A7C447D00267924 /* SSignal.m in Sources */, D087632C1A839EE800632240 /* SDisposableSet.m in Sources */, D0445E861A7C447D00267924 /* SSubscriber.m in Sources */, - D0445E7E1A7C447D00267924 /* SSignal+Concat.m in Sources */, D0445E831A7C447D00267924 /* SSignal+SideEffects.m in Sources */, D0445E811A7C447D00267924 /* SSignal+Meta.m in Sources */, D0445E7D1A7C447D00267924 /* SSignal+Combine.m in Sources */, diff --git a/SSignalKit/SDisposableSet.m b/SSignalKit/SDisposableSet.m index caabf076ea..469436ad99 100644 --- a/SSignalKit/SDisposableSet.m +++ b/SSignalKit/SDisposableSet.m @@ -5,6 +5,7 @@ @interface SDisposableSet () { OSSpinLock _lock; + bool _disposed; id _singleDisposable; NSArray *_multipleDisposables; } @@ -18,24 +19,33 @@ if (disposable == nil) return; + bool dispose = false; + OSSpinLockLock(&_lock); - if (_multipleDisposables != nil) + dispose = _disposed; + if (!dispose) { - NSMutableArray *multipleDisposables = [[NSMutableArray alloc] initWithArray:_multipleDisposables]; - [multipleDisposables addObject:disposable]; - _multipleDisposables = multipleDisposables; - } - else if (_singleDisposable != nil) - { - NSMutableArray *multipleDisposables = [[NSMutableArray alloc] initWithObjects:_singleDisposable, disposable, nil]; - _multipleDisposables = multipleDisposables; - _singleDisposable = nil; - } - else - { - _singleDisposable = disposable; + if (_multipleDisposables != nil) + { + NSMutableArray *multipleDisposables = [[NSMutableArray alloc] initWithArray:_multipleDisposables]; + [multipleDisposables addObject:disposable]; + _multipleDisposables = multipleDisposables; + } + else if (_singleDisposable != nil) + { + NSMutableArray *multipleDisposables = [[NSMutableArray alloc] initWithObjects:_singleDisposable, disposable, nil]; + _multipleDisposables = multipleDisposables; + _singleDisposable = nil; + } + else + { + _singleDisposable = disposable; + } } OSSpinLockUnlock(&_lock); + + if (dispose) + [disposable dispose]; } - (void)dispose @@ -44,10 +54,14 @@ NSArray *multipleDisposables = nil; OSSpinLockLock(&_lock); - singleDisposable = _singleDisposable; - multipleDisposables = _multipleDisposables; - _singleDisposable = nil; - _multipleDisposables = nil; + if (!_disposed) + { + _disposed = true; + singleDisposable = _singleDisposable; + multipleDisposables = _multipleDisposables; + _singleDisposable = nil; + _multipleDisposables = nil; + } OSSpinLockUnlock(&_lock); if (singleDisposable != nil) diff --git a/SSignalKit/SMetaDisposable.m b/SSignalKit/SMetaDisposable.m index 4e461eed3f..4e9c8e4fab 100644 --- a/SSignalKit/SMetaDisposable.m +++ b/SSignalKit/SMetaDisposable.m @@ -4,54 +4,50 @@ @interface SMetaDisposable () { - void *_disposable; + OSSpinLock _lock; + bool _disposed; + id _disposable; } @end @implementation SMetaDisposable -- (void)dealloc -{ - while (true) - { - void *previousDisposable = _disposable; - if (OSAtomicCompareAndSwapPtr(previousDisposable, NULL, &_disposable)) - { - if (previousDisposable != NULL) - { - __strong id strongPreviousDisposable = (__bridge_transfer id)previousDisposable; - strongPreviousDisposable = nil; - } - - break; - } - } -} - - (void)setDisposable:(id)disposable { - void *newDisposable = (__bridge_retained void *)disposable; - while (true) + id previousDisposable = nil; + bool dispose = false; + + OSSpinLockLock(&_lock); + dispose = _disposed; + if (!dispose) { - void *previousDisposable = _disposable; - if (OSAtomicCompareAndSwapPtr(previousDisposable, newDisposable, &_disposable)) - { - if (previousDisposable != NULL) - { - __strong id strongPreviousDisposable = (__bridge_transfer id)previousDisposable; - [strongPreviousDisposable dispose]; - strongPreviousDisposable = nil; - } - - break; - } + previousDisposable = _disposable; + _disposable = disposable; } + OSSpinLockUnlock(&_lock); + + if (previousDisposable != nil) + [previousDisposable dispose]; + + if (dispose) + [disposable dispose]; } - (void)dispose { - [self setDisposable:nil]; + id disposable = nil; + + OSSpinLockLock(&_lock); + if (!_disposed) + { + disposable = _disposable; + _disposed = true; + } + OSSpinLockUnlock(&_lock); + + if (disposable != nil) + [disposable dispose]; } @end diff --git a/SSignalKit/SSignal+Concat.h b/SSignalKit/SSignal+Concat.h deleted file mode 100644 index 356f80c213..0000000000 --- a/SSignalKit/SSignal+Concat.h +++ /dev/null @@ -1,5 +0,0 @@ -#import "SSignal.h" - -@interface SSignal (Concat) - -@end diff --git a/SSignalKit/SSignal+Concat.m b/SSignalKit/SSignal+Concat.m deleted file mode 100644 index 8d76e5159d..0000000000 --- a/SSignalKit/SSignal+Concat.m +++ /dev/null @@ -1,14 +0,0 @@ -#import "SSignal+Concat.h" - -#import "SAtomic.h" - -@implementation SSignal (Concat) - -- (SSignal *)concat:(SSignal *)another -{ - NSAssert(false, @"123"); - - return nil; -} - -@end diff --git a/SSignalKit/SSignal+Multicast.m b/SSignalKit/SSignal+Multicast.m index 6e08423750..3fe3a6f789 100644 --- a/SSignalKit/SSignal+Multicast.m +++ b/SSignalKit/SSignal+Multicast.m @@ -91,7 +91,18 @@ typedef enum { for (SSubscriber *subscriber in currentSubscribers) { - [subscriber putEvent:event]; + switch (event.type) + { + case SEventTypeNext: + [subscriber putNext:event.data]; + break; + case SEventTypeError: + [subscriber putError:event.data]; + break; + case SEventTypeCompleted: + [subscriber putCompletion]; + break; + } } } diff --git a/SSignalKit/SSignal.m b/SSignalKit/SSignal.m index c86a9252fb..1cae763a69 100644 --- a/SSignalKit/SSignal.m +++ b/SSignalKit/SSignal.m @@ -1,5 +1,7 @@ #import "SSignal.h" +#import "SBlockDisposable.h" + @interface SSignal () { } @@ -23,7 +25,11 @@ SSubscriber *subscriber = [[SSubscriber alloc] initWithNext:next error:error completed:completed]; id disposable = _generator(subscriber); [subscriber _assignDisposable:disposable]; - return disposable; + return [[SBlockDisposable alloc] initWithBlock:^ + { + [subscriber _markTerminatedWithoutDisposal]; + [disposable dispose]; + }]; } - (id)startWithNext:(void (^)(id next))next @@ -31,7 +37,7 @@ SSubscriber *subscriber = [[SSubscriber alloc] initWithNext:next error:nil completed:nil]; id disposable = _generator(subscriber); [subscriber _assignDisposable:disposable]; - return disposable; + return subscriber; } - (id)startWithNext:(void (^)(id next))next completed:(void (^)())completed @@ -39,7 +45,7 @@ SSubscriber *subscriber = [[SSubscriber alloc] initWithNext:next error:nil completed:completed]; id disposable = _generator(subscriber); [subscriber _assignDisposable:disposable]; - return disposable; + return subscriber; } @end diff --git a/SSignalKit/SSubscriber.h b/SSignalKit/SSubscriber.h index 896917ded7..2689f6c566 100644 --- a/SSignalKit/SSubscriber.h +++ b/SSignalKit/SSubscriber.h @@ -12,8 +12,8 @@ - (instancetype)initWithNext:(void (^)(id))next error:(void (^)(id))error completed:(void (^)())completed; - (void)_assignDisposable:(id)disposable; +- (void)_markTerminatedWithoutDisposal; -- (void)putEvent:(SEvent *)event; - (void)putNext:(id)next; - (void)putError:(id)error; - (void)putCompletion; diff --git a/SSignalKit/SSubscriber.m b/SSignalKit/SSubscriber.m index 43f6d3a6f4..ef5979d5db 100644 --- a/SSignalKit/SSubscriber.m +++ b/SSignalKit/SSubscriber.m @@ -1,16 +1,11 @@ #import "SSubscriber.h" -#import - -#import "SEvent.h" - -#define lockSelf(x) pthread_mutex_lock(&x->_mutex) -#define unlockSelf(x) pthread_mutex_unlock(&x->_mutex) +#import @interface SSubscriber () { - pthread_mutex_t _mutex; - + OSSpinLock _lock; + bool _terminated; id _disposable; } @@ -23,7 +18,6 @@ self = [super init]; if (self != nil) { - pthread_mutex_init(&_mutex, NULL); _next = [next copy]; _error = [error copy]; _completed = [completed copy]; @@ -36,53 +30,27 @@ _disposable = disposable; } -- (void)putEvent:(SEvent *)event +- (void)_markTerminatedWithoutDisposal { - bool shouldDispose = false; - void (^next)(id) = nil; - void (^error)(id) = nil; - void (^completed)(id) = nil; - - lockSelf(self); - next = self->_next; - error = self->_error; - completed = self->_completed; - if (event.type != SEventTypeNext) + OSSpinLockLock(&_lock); + if (!_terminated) { - shouldDispose = true; - self->_next = nil; - self->_error = nil; - self->_completed = nil; + _terminated = true; + _next = nil; + _error = nil; + _completed = nil; } - unlockSelf(self); - - switch (event.type) - { - case SEventTypeNext: - if (next) - next(event.data); - break; - case SEventTypeError: - if (error) - error(event.data); - break; - case SEventTypeCompleted: - if (completed) - completed(event.data); - break; - } - - if (shouldDispose) - [self->_disposable dispose]; + OSSpinLockUnlock(&_lock); } - (void)putNext:(id)next { void (^fnext)(id) = nil; - lockSelf(self); - fnext = self->_next; - unlockSelf(self); + OSSpinLockLock(&_lock); + if (!_terminated) + fnext = self->_next; + OSSpinLockUnlock(&_lock); if (fnext) fnext(next); @@ -93,13 +61,17 @@ bool shouldDispose = false; void (^ferror)(id) = nil; - lockSelf(self); - ferror = self->_error; - shouldDispose = true; - self->_next = nil; - self->_error = nil; - self->_completed = nil; - unlockSelf(self); + OSSpinLockLock(&_lock); + if (!_terminated) + { + ferror = self->_error; + shouldDispose = true; + self->_next = nil; + self->_error = nil; + self->_completed = nil; + _terminated = true; + } + OSSpinLockUnlock(&_lock); if (ferror) ferror(error); @@ -113,13 +85,17 @@ bool shouldDispose = false; void (^completed)() = nil; - lockSelf(self); - completed = self->_completed; - shouldDispose = true; - self->_next = nil; - self->_error = nil; - self->_completed = nil; - unlockSelf(self); + OSSpinLockLock(&_lock); + if (!_terminated) + { + completed = self->_completed; + shouldDispose = true; + self->_next = nil; + self->_error = nil; + self->_completed = nil; + _terminated = true; + } + OSSpinLockUnlock(&_lock); if (completed) completed(); diff --git a/SSignalKitTests/SDisposableTests.m b/SSignalKitTests/SDisposableTests.m index f071376688..e04fc32049 100644 --- a/SSignalKitTests/SDisposableTests.m +++ b/SSignalKitTests/SDisposableTests.m @@ -241,4 +241,74 @@ XCTAssertFalse(disposed2); } +- (void)testMetaDisposableAlreadyDisposed +{ + bool deallocated1 = false; + __block bool disposed1 = false; + bool deallocated2 = false; + __block bool disposed2 = false; + + @autoreleasepool + { + DeallocatingObject *object1 = [[DeallocatingObject alloc] initWithDeallocated:&deallocated1]; + dispatch_block_t block1 = ^{ + [object1 description]; + disposed1 = true; + }; + SBlockDisposable *blockDisposable1 = [[SBlockDisposable alloc] initWithBlock:[block1 copy]]; + + DeallocatingObject *object2 = [[DeallocatingObject alloc] initWithDeallocated:&deallocated2]; + dispatch_block_t block2 = ^{ + [object2 description]; + disposed2 = true; + }; + SBlockDisposable *blockDisposable2 = [[SBlockDisposable alloc] initWithBlock:[block2 copy]]; + + SMetaDisposable *metaDisposable = [[SMetaDisposable alloc] init]; + [metaDisposable setDisposable:blockDisposable1]; + [metaDisposable dispose]; + [metaDisposable setDisposable:blockDisposable2]; + } + + XCTAssertTrue(deallocated1); + XCTAssertTrue(disposed1); + XCTAssertTrue(deallocated2); + XCTAssertTrue(disposed2); +} + +- (void)testDisposableSetAlreadyDisposed +{ + bool deallocated1 = false; + __block bool disposed1 = false; + bool deallocated2 = false; + __block bool disposed2 = false; + + @autoreleasepool + { + DeallocatingObject *object1 = [[DeallocatingObject alloc] initWithDeallocated:&deallocated1]; + dispatch_block_t block1 = ^{ + [object1 description]; + disposed1 = true; + }; + SBlockDisposable *blockDisposable1 = [[SBlockDisposable alloc] initWithBlock:[block1 copy]]; + + DeallocatingObject *object2 = [[DeallocatingObject alloc] initWithDeallocated:&deallocated2]; + dispatch_block_t block2 = ^{ + [object2 description]; + disposed2 = true; + }; + SBlockDisposable *blockDisposable2 = [[SBlockDisposable alloc] initWithBlock:[block2 copy]]; + + SMetaDisposable *metaDisposable = [[SMetaDisposable alloc] init]; + [metaDisposable setDisposable:blockDisposable1]; + [metaDisposable dispose]; + [metaDisposable setDisposable:blockDisposable2]; + } + + XCTAssertTrue(deallocated1); + XCTAssertTrue(disposed1); + XCTAssertTrue(deallocated2); + XCTAssertTrue(disposed2); +} + @end diff --git a/SSignalKitTests/SSignalBasicTests.m b/SSignalKitTests/SSignalBasicTests.m index 90857ed6b1..be66425d44 100644 --- a/SSignalKitTests/SSignalBasicTests.m +++ b/SSignalKitTests/SSignalBasicTests.m @@ -173,31 +173,29 @@ __block bool disposed = false; __block bool generated = false; + @autoreleasepool { - @autoreleasepool + DeallocatingObject *object = [[DeallocatingObject alloc] initWithDeallocated:&deallocated]; + SSignal *signal = [[[SSignal alloc] initWithGenerator:^id(SSubscriber *subscriber) { - DeallocatingObject *object = [[DeallocatingObject alloc] initWithDeallocated:&deallocated]; - SSignal *signal = [[[SSignal alloc] initWithGenerator:^id(SSubscriber *subscriber) - { - [subscriber putNext:@1]; - [object description]; - return [[SBlockDisposable alloc] initWithBlock:^ - { - [object description]; - disposed = true; - }]; - }] _mapInplace:^id(id value) + [subscriber putNext:@1]; + [object description]; + return [[SBlockDisposable alloc] initWithBlock:^ { [object description]; - return @([value intValue] * 2); + disposed = true; }]; - - id disposable = [signal startWithNext:^(id value) - { - generated = [value isEqual:@2]; - } error:nil completed:nil]; - [disposable dispose]; - } + }] _mapInplace:^id(id value) + { + [object description]; + return @([value intValue] * 2); + }]; + + id disposable = [signal startWithNext:^(id value) + { + generated = [value isEqual:@2]; + } error:nil completed:nil]; + [disposable dispose]; } XCTAssertTrue(deallocated); @@ -205,4 +203,41 @@ XCTAssertTrue(generated); } +- (void)testSubscriberDisposal +{ + __block bool disposed = false; + __block bool generated = false; + + dispatch_queue_t queue = dispatch_queue_create(NULL, 0); + + @autoreleasepool + { + SSignal *signal = [[SSignal alloc] initWithGenerator:^id(SSubscriber *subscriber) + { + dispatch_async(queue, ^ + { + [subscriber putNext:@1]; + }); + + return [[SBlockDisposable alloc] initWithBlock:^ + { + disposed = true; + }]; + }]; + + id disposable = [signal startWithNext:^(id value) + { + generated = true; + } error:nil completed:nil]; + [disposable dispose]; + } + + dispatch_barrier_sync(queue, ^ + { + }); + + XCTAssertTrue(disposed); + XCTAssertFalse(generated); +} + @end