- 
                Notifications
    You must be signed in to change notification settings 
- Fork 712
Update our locks to use Synchronization.Mutex or os_unfair_lock #3428
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Now that we require Swift 6.0, update `NIOLock` and `NIOLockedValueBox` to use `Synchronization.Mutex`. Additionally, move all `pthread_mutex_t` and `SRWLOCK` code into `lock.swift` to support our condition variable type. Results: - Much simpler code - Better or equal performance, depending on the platform
Update NIOLock to use Synchronization.Mutex
| var mutex: os_unfair_lock_s | ||
| #else | ||
| @usableFromInline | ||
| let mutex: Mutex<Void> | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the Mutex case, let's put the storage into the Mutex as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we do that, we can't implement NIOLockedValueBox.Unsafe; the only public APIs on Mutex<non Void> are the two withLock methods.
We could use Synchronization._MutexHandle instead of Mutex<Void>, if you prefer.  _MutexHandle is public and has public locking methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh that's unfortunate. I'd like us to avoid using underscored parts of Swift to the greatest extent possible.
| #if canImport(Darwin) | ||
| let mutex_ptr = _getUnsafePointerToStoredProperties(self).assumingMemoryBound(to: os_unfair_lock_s.self) | ||
| os_unfair_lock_unlock(mutex_ptr) | ||
| _fixLifetime(self) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's avoid _fixLifetime. We can use withExtendedLifetime(self) { } which has an awkward spelling but avoids the underscored field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I changed it.
Now that we require Swift 6.0, update
LockStorageto useSynchronization.Mutexoros_unfair_lockMotivation:
Simplify and optimize
NIOLockandNIOLockedValueBoxon every platformModifications:
pthread_mutex_tandSRWLOCKcode intolock.swift, to support our condition variable typeLockStoragearoundSynchronization.Mutexon most platformsos_unfair_lockinstead to support back deploymentManagedAtomicfromswift-atomicsResult: