Merge pull request #187 from bignerdranch/zwaldowski/swift-3_2-tsan

Avoid using Swift inout to guarantee C pointer semantics for atomics

Author: zwaldowski

.fastlane/Fastfile

@@ -65,8 +65,4 @@ platform :ios do
     test
     build_for_other_platforms
   end
-
-  error do |lane, exception|
-    sh "cat ~/Library/Logs/DiagnosticReports/xctest*.crash | true"
-  end
 end

.travis.yml

@@ -15,6 +15,9 @@ matrix:
   include:
     - script: fastlane ios ci
       env: JOB=iOS
+      after_failure:
+        - cat ~/Library/Developer/Xcode/DerivedData/Deferred-*/Logs/Test/*/Session-MobileDeferredTests-*.log | true
+        - cat ~/Library/Logs/scan/Deferred-*.log | true
     - language: generic
       os: linux
       dist: trusty

Sources/Atomics/include/Atomics.h

@@ -18,14 +18,6 @@
 #endif // !__APPLE__
 #include <pthread.h>
 
-#if !defined(OS_INLINE)
-#if __GNUC__
-#define OS_INLINE static __inline__
-#else
-#define OS_INLINE
-#endif // !__GNUC__
-#endif // !OS_INLINE
-
 #if !defined(OS_ALWAYS_INLINE)
 #if __GNUC__
 #define OS_ALWAYS_INLINE __attribute__((__always_inline__))
@@ -60,8 +52,8 @@ typedef struct {
     } __impl;
 } bnr_spinlock_t;
 
-OS_INLINE OS_ALWAYS_INLINE OS_SWIFT_NAME(UnsafeNativeLock.setup(self:))
-void bnr_native_lock_create(bnr_spinlock_t *_Nonnull address) {
+OS_ALWAYS_INLINE
+static inline void bnr_native_lock_init(bnr_spinlock_t *_Nonnull address) {
 #if defined(__APPLE__)
     if (&os_unfair_lock_trylock != NULL) {
         address->__impl.modern = OS_UNFAIR_LOCK_INIT;
@@ -72,8 +64,8 @@ void bnr_native_lock_create(bnr_spinlock_t *_Nonnull address) {
     pthread_mutex_init(&address->__impl.legacy, NULL);
 }
 
-OS_INLINE OS_ALWAYS_INLINE OS_SWIFT_NAME(UnsafeNativeLock.invalidate(self:))
-void bnr_native_lock_destroy(bnr_spinlock_t *_Nonnull address) {
+OS_ALWAYS_INLINE
+static inline void bnr_native_lock_destroy(bnr_spinlock_t *_Nonnull address) {
 #if defined(__APPLE__)
     if (&os_unfair_lock_trylock != NULL) {
         return;
@@ -83,8 +75,8 @@ void bnr_native_lock_destroy(bnr_spinlock_t *_Nonnull address) {
     pthread_mutex_destroy(&address->__impl.legacy);
 }
 
-OS_INLINE OS_ALWAYS_INLINE OS_SWIFT_NAME(UnsafeNativeLock.lock(self:))
-void bnr_native_lock_lock(bnr_spinlock_t *_Nonnull address) {
+OS_ALWAYS_INLINE
+static inline void bnr_native_lock_lock(bnr_spinlock_t *_Nonnull address) {
 #if defined(__APPLE__)
     if (&os_unfair_lock_lock != NULL) {
         return os_unfair_lock_lock(&address->__impl.modern);
@@ -94,8 +86,8 @@ void bnr_native_lock_lock(bnr_spinlock_t *_Nonnull address) {
     pthread_mutex_lock(&address->__impl.legacy);
 }
 
-OS_INLINE OS_ALWAYS_INLINE OS_SWIFT_NAME(UnsafeNativeLock.try(self:))
-bool bnr_native_lock_trylock(bnr_spinlock_t *_Nonnull address) {
+OS_ALWAYS_INLINE
+static inline bool bnr_native_lock_trylock(bnr_spinlock_t *_Nonnull address) {
 #if defined(__APPLE__)
     if (&os_unfair_lock_trylock != NULL) {
         return os_unfair_lock_trylock(&address->__impl.modern);
@@ -105,8 +97,8 @@ bool bnr_native_lock_trylock(bnr_spinlock_t *_Nonnull address) {
     return pthread_mutex_trylock(&address->__impl.legacy) == 0;
 }
 
-OS_INLINE OS_ALWAYS_INLINE OS_SWIFT_NAME(UnsafeNativeLock.unlock(self:))
-void bnr_native_lock_unlock(bnr_spinlock_t *_Nonnull address) {
+OS_ALWAYS_INLINE
+static inline void bnr_native_lock_unlock(bnr_spinlock_t *_Nonnull address) {
 #if defined(__APPLE__)
     if (&os_unfair_lock_unlock != NULL) {
         return os_unfair_lock_unlock(&address->__impl.modern);
@@ -121,13 +113,13 @@ typedef struct {
     _Atomic(void *_Nullable) value;
 } bnr_atomic_ptr_t;
 
-OS_INLINE OS_ALWAYS_INLINE OS_SWIFT_NAME(UnsafeAtomicRawPointer.load(self:order:))
-void *_Nullable bnr_atomic_ptr_load(volatile bnr_atomic_ptr_t *_Nonnull target, bnr_atomic_memory_order_t order) {
+OS_ALWAYS_INLINE
+static inline void *_Nullable bnr_atomic_ptr_load(volatile bnr_atomic_ptr_t *_Nonnull target, bnr_atomic_memory_order_t order) {
     return __c11_atomic_load(&target->value, order);
 }
 
-OS_INLINE OS_ALWAYS_INLINE OS_SWIFT_NAME(UnsafeAtomicRawPointer.compareAndSwap(self:from:to:order:))
-bool bnr_atomic_ptr_compare_and_swap(volatile bnr_atomic_ptr_t *_Nonnull target, void *_Nullable expected, void *_Nullable desired, bnr_atomic_memory_order_t order) {
+OS_ALWAYS_INLINE
+static inline bool bnr_atomic_ptr_compare_and_swap(volatile bnr_atomic_ptr_t *_Nonnull target, void *_Nullable expected, void *_Nullable desired, bnr_atomic_memory_order_t order) {
     return __c11_atomic_compare_exchange_strong(&target->value, &expected, desired, __ATOMIC_ACQ_REL, __ATOMIC_RELAXED);
 }
 
@@ -136,13 +128,13 @@ typedef struct {
     _Atomic(bool) value;
 } bnr_atomic_flag_t;
 
-OS_INLINE OS_ALWAYS_INLINE OS_SWIFT_NAME(UnsafeAtomicBool.testAndSet(self:))
-bool bnr_atomic_flag_test_and_set(volatile bnr_atomic_flag_t *_Nonnull target) {
+OS_ALWAYS_INLINE
+static inline bool bnr_atomic_flag_test_and_set(volatile bnr_atomic_flag_t *_Nonnull target) {
     return __c11_atomic_exchange(&target->value, 1, __ATOMIC_RELAXED);
 }
 
-OS_INLINE OS_ALWAYS_INLINE OS_SWIFT_NAME(UnsafeAtomicBool.test(self:))
-bool bnr_atomic_flag_test(volatile bnr_atomic_flag_t *_Nonnull target) {
+OS_ALWAYS_INLINE
+static inline bool bnr_atomic_flag_test(volatile bnr_atomic_flag_t *_Nonnull target) {
     return __c11_atomic_load(&target->value, __ATOMIC_RELAXED);
 }
 
@@ -151,23 +143,23 @@ typedef struct {
     _Atomic(uint_fast8_t) value;
 } bnr_atomic_bitmask_t;
 
-OS_INLINE OS_ALWAYS_INLINE OS_SWIFT_NAME(UnsafeAtomicBitmask.setInitialValue(self:_:))
-void bnr_atomic_bitmask_init(volatile bnr_atomic_bitmask_t *_Nonnull target, uint_fast8_t value) {
+OS_ALWAYS_INLINE
+static inline void bnr_atomic_bitmask_init(volatile bnr_atomic_bitmask_t *_Nonnull target, uint_fast8_t value) {
     __c11_atomic_init(&target->value, value);
 }
 
-OS_INLINE OS_ALWAYS_INLINE OS_SWIFT_NAME(UnsafeAtomicBitmask.or(self:with:order:))
-uint_fast8_t bnr_atomic_bitmask_or(volatile bnr_atomic_bitmask_t *_Nonnull target, uint_fast8_t value, bnr_atomic_memory_order_t order) {
+OS_ALWAYS_INLINE
+static inline uint_fast8_t bnr_atomic_bitmask_or(volatile bnr_atomic_bitmask_t *_Nonnull target, uint_fast8_t value, bnr_atomic_memory_order_t order) {
     return __c11_atomic_fetch_or(&target->value, value, order);
 }
 
-OS_INLINE OS_ALWAYS_INLINE OS_SWIFT_NAME(UnsafeAtomicBitmask.and(self:with:order:))
-uint_fast8_t bnr_atomic_bitmask_and(volatile bnr_atomic_bitmask_t *_Nonnull target, uint_fast8_t value, bnr_atomic_memory_order_t order) {
+OS_ALWAYS_INLINE
+static inline uint_fast8_t bnr_atomic_bitmask_and(volatile bnr_atomic_bitmask_t *_Nonnull target, uint_fast8_t value, bnr_atomic_memory_order_t order) {
     return __c11_atomic_fetch_and(&target->value, value, order);
 }
 
-OS_INLINE OS_ALWAYS_INLINE OS_SWIFT_NAME(UnsafeAtomicBitmask.test(self:for:))
-bool bnr_atomic_bitmask_test(const bnr_atomic_bitmask_t *_Nonnull target, uint_fast8_t value) {
+OS_ALWAYS_INLINE
+static inline bool bnr_atomic_bitmask_test(const bnr_atomic_bitmask_t *_Nonnull target, uint_fast8_t value) {
     return (__c11_atomic_load((_Atomic(uint_fast8_t) *)&target->value, __ATOMIC_RELAXED) & value) != 0;
 }
 
@@ -176,18 +168,18 @@ typedef struct {
     _Atomic(int_fast32_t) value;
 } bnr_atomic_counter_t;
 
-OS_INLINE OS_ALWAYS_INLINE OS_SWIFT_NAME(UnsafeAtomicCounter.increment(self:))
-int_fast32_t bnr_atomic_counter_increment(volatile bnr_atomic_counter_t *_Nonnull target) {
+OS_ALWAYS_INLINE
+static inline int_fast32_t bnr_atomic_counter_increment(volatile bnr_atomic_counter_t *_Nonnull target) {
     return __c11_atomic_fetch_add(&target->value, 1, __ATOMIC_SEQ_CST) + 1;
 }
 
-OS_INLINE OS_ALWAYS_INLINE OS_SWIFT_NAME(UnsafeAtomicCounter.decrement(self:))
-int_fast32_t bnr_atomic_counter_decrement(volatile bnr_atomic_counter_t *_Nonnull target) {
+OS_ALWAYS_INLINE
+static inline int_fast32_t bnr_atomic_counter_decrement(volatile bnr_atomic_counter_t *_Nonnull target) {
     return __c11_atomic_fetch_sub(&target->value, 1, __ATOMIC_SEQ_CST) - 1;
 }
 
-OS_INLINE OS_ALWAYS_INLINE OS_SWIFT_NAME(UnsafeAtomicCounter.load(self:))
-int_fast32_t bnr_atomic_counter_load(volatile bnr_atomic_counter_t *_Nonnull target) {
+OS_ALWAYS_INLINE
+static inline int_fast32_t bnr_atomic_counter_load(volatile bnr_atomic_counter_t *_Nonnull target) {
     return __c11_atomic_load(&target->value, __ATOMIC_SEQ_CST);
 }
 

Sources/Deferred/Deferred.swift

@@ -54,7 +54,7 @@ public final class Deferred<Value>: FutureProtocol, PromiseProtocol {
 
     private func notify(flags: DispatchWorkItemFlags, upon queue: DispatchQueue, execute body: @escaping(Value) -> Void) {
         group.notify(flags: flags, queue: queue) { [storage] in
-            guard let ptr = storage.withAtomicPointerToElement({ $0.load(order: .none) }) else { return }
+            guard let ptr = storage.withAtomicPointerToElement({ bnr_atomic_ptr_load($0, .none) }) else { return }
             body(Storage.unbox(from: ptr))
         }
     }
@@ -79,7 +79,7 @@ public final class Deferred<Value>: FutureProtocol, PromiseProtocol {
 
     public func wait(until time: DispatchTime) -> Value? {
         guard case .success = group.wait(timeout: time),
-            let ptr = storage.withAtomicPointerToElement({ $0.load(order: .none) }) else { return nil }
+            let ptr = storage.withAtomicPointerToElement({ bnr_atomic_ptr_load($0, .none) }) else { return nil }
 
         return Storage.unbox(from: ptr)
     }
@@ -88,7 +88,7 @@ public final class Deferred<Value>: FutureProtocol, PromiseProtocol {
 
     public var isFilled: Bool {
         return storage.withAtomicPointerToElement {
-            $0.load(order: .none) != nil
+            bnr_atomic_ptr_load($0, .none) != nil
         }
     }
 
@@ -97,7 +97,7 @@ public final class Deferred<Value>: FutureProtocol, PromiseProtocol {
         let box = Storage.box(value)
 
         let wonRace = storage.withAtomicPointerToElement {
-            $0.compareAndSwap(from: nil, to: box.toOpaque(), order: .thread)
+            bnr_atomic_ptr_compare_and_swap($0, nil, box.toOpaque(), .thread)
         }
 
         if wonRace {
@@ -135,16 +135,14 @@ private final class DeferredStorage<Value>: ManagedBuffer<Void, DeferredRaw<Valu
         return unsafeDowncast(super.create(minimumCapacity: 1, makingHeaderWith: { _ in }), to: _Self.self)
     }
 
-    func withAtomicPointerToElement<Return>(_ body: (inout UnsafeAtomicRawPointer) throws -> Return) rethrows -> Return {
+    func withAtomicPointerToElement<Return>(_ body: (UnsafeMutablePointer<UnsafeAtomicRawPointer>) throws -> Return) rethrows -> Return {
         return try withUnsafeMutablePointerToElements { target in
-            try target.withMemoryRebound(to: UnsafeAtomicRawPointer.self, capacity: 1) { (atomicPointertoElement) in
-                try body(&atomicPointertoElement.pointee)
-            }
+            try target.withMemoryRebound(to: UnsafeAtomicRawPointer.self, capacity: 1, body)
         }
     }
 
     deinit {
-        guard let ptr = withAtomicPointerToElement({ $0.load(order: .global) }) else { return }
+        guard let ptr = withAtomicPointerToElement({ bnr_atomic_ptr_load($0, .global) }) else { return }
         Element.fromOpaque(ptr).release()
     }
 

Sources/Deferred/Locking.swift

@@ -70,22 +70,22 @@ public final class NativeLock: Locking, MaybeLocking {
 
     /// Creates a standard platform lock.
     public init() {
-        lock.setup()
+        bnr_native_lock_init(&lock)
     }
 
     deinit {
-        lock.invalidate()
+        bnr_native_lock_destroy(&lock)
     }
 
     public func withReadLock<Return>(_ body: () throws -> Return) rethrows -> Return {
-        lock.lock()
-        defer { lock.unlock() }
+        bnr_native_lock_lock(&lock)
+        defer { bnr_native_lock_unlock(&lock) }
         return try body()
     }
 
     public func withAttemptedReadLock<Return>(_ body: () -> Return) -> Return? {
-        guard lock.try() else { return nil }
-        defer { lock.unlock() }
+        guard bnr_native_lock_trylock(&lock) else { return nil }
+        defer { bnr_native_lock_unlock(&lock) }
         return body()
     }
 

Sources/Task/ExistentialTask.swift

@@ -127,7 +127,7 @@ extension Task {
 
 #if !os(macOS) && !os(iOS) && !os(tvOS) && !os(watchOS)
     fileprivate func markCancelled() {
-        _ = rawIsCancelled.testAndSet()
+        _ = bnr_atomic_flag_test_and_set(&rawIsCancelled)
     }
 #endif
 
@@ -136,7 +136,7 @@ extension Task {
 #if os(macOS) || os(iOS) || os(tvOS) || os(watchOS)
         return progress.isCancelled
 #else
-        return rawIsCancelled.test()
+        return bnr_atomic_flag_test(&rawIsCancelled)
 #endif
     }
 }

Sources/Task/TaskProgress.swift

@@ -117,7 +117,7 @@ private final class ProxyProgress: Progress {
 
         init(observing observee: Progress, observer: ProxyProgress) {
             self.observer = observer
-            state.setInitialValue(State.ready.rawValue)
+            bnr_atomic_bitmask_init(&state, State.ready.rawValue)
             super.init()
 
             for key in Observation.attributes {
@@ -126,13 +126,13 @@ private final class ProxyProgress: Progress {
             observee.addObserver(self, forKeyPath: #keyPath(Progress.cancelled), options: Observation.options, context: &Observation.cancelledContext)
             observee.addObserver(self, forKeyPath: #keyPath(Progress.paused), options: Observation.options, context: &Observation.pausedContext)
 
-            state.or(with: State.observing.rawValue, order: .write)
+            bnr_atomic_bitmask_or(&state, State.observing.rawValue, .write)
         }
 
         func cancel(observing observee: Progress) {
-            let oldState = State(rawValue: state.and(with: ~State.ready.rawValue, order: .none))
+            let oldState = State(rawValue: bnr_atomic_bitmask_and(&state, ~State.ready.rawValue, .none))
             guard !oldState.isStrictSuperset(of: .cancellable) else { return }
-            state.or(with: State.cancelled.rawValue, order: .none)
+            bnr_atomic_bitmask_or(&state, State.cancelled.rawValue, .none)
 
             for key in Observation.attributes {
                 observee.removeObserver(self, forKeyPath: key, context: &Observation.attributesContext)
@@ -142,7 +142,7 @@ private final class ProxyProgress: Progress {
         }
 
         override func observeValue(forKeyPath keyPath: String?, of object: Any?, change: [NSKeyValueChangeKey : Any]?, context: UnsafeMutableRawPointer?) {
-            guard let keyPath = keyPath, object != nil, state.test(for: State.ready.rawValue), let observer = observer, let newValue = change?[.newKey] else { return }
+            guard let keyPath = keyPath, object != nil, bnr_atomic_bitmask_test(&state, State.ready.rawValue), let observer = observer, let newValue = change?[.newKey] else { return }
             switch context {
             case (&Observation.cancelledContext)?:
                 // Gotta trust KVO a little
@@ -239,7 +239,7 @@ extension Progress {
                 progress.totalUnitCount = 1
                 progress.completedUnitCount = 1
             }
-            
+
             return progress
         }
     }
@@ -317,3 +317,4 @@ extension Task {
 }
 
 #endif
+

Tests/DeferredTests/FutureTests.swift

@@ -112,7 +112,7 @@ class FutureTests: XCTestCase {
         var counter = UnsafeAtomicCounter()
 
         let mapped = d.every { (value) -> (Int) in
-            counter.increment()
+            bnr_atomic_counter_increment(&counter)
             return value * 2
         }
 
@@ -125,6 +125,6 @@ class FutureTests: XCTestCase {
 
         XCTAssertEqual(mapped.waitShort(), 2)
 
-        XCTAssertEqual(counter.load(), 2)
+        XCTAssertEqual(bnr_atomic_counter_load(&counter), 2)
     }
 }