Skip to content

Commit 66b9d21

Browse files
committed
[Runtime] Remove all use of read/write locks.
Read/write locks are not as good as you'd think; a simple mutex is better in almost all cases. rdar://90776105
1 parent 0cf687a commit 66b9d21

File tree

11 files changed

+28
-1072
lines changed

11 files changed

+28
-1072
lines changed

include/swift/Runtime/Mutex.h

+7-302
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
//===--- Mutex.h - Mutex and ReadWriteLock ----------------------*- C++ -*-===//
1+
//===--- Mutex.h - Mutex ----------------------------------------*- C++ -*-===//
22
//
33
// This source file is part of the Swift.org open source project
44
//
@@ -10,14 +10,18 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212
//
13-
// Mutex, ReadWriteLock, and Scoped lock abstractions for use in
14-
// Swift runtime.
13+
// Mutex and Scoped lock abstractions for use in Swift runtime.
1514
//
1615
// We intentionally do not provide a condition-variable abstraction.
1716
// Traditional condition-variable interfaces are subject to unavoidable
1817
// priority inversions, as well as making poor use of threads.
1918
// Prefer AtomicWaitQueue.
2019
//
20+
// We also intentionally avoid read/write locks. It's difficult to implement a
21+
// performant and fair read/write lock, and indeed many common implementations
22+
// rely on condition variables, which, again, are subject to unavoidable
23+
// priority inversions.
24+
//
2125
//===----------------------------------------------------------------------===//
2226

2327
#ifndef SWIFT_RUNTIME_MUTEX_H
@@ -167,238 +171,6 @@ class Mutex {
167171
MutexHandle Handle;
168172
};
169173

170-
/// Compile time adjusted stack based object that locks/unlocks the supplied
171-
/// ReadWriteLock type. Use the provided typedefs instead of this directly.
172-
template <typename T, bool Read, bool Inverted> class ScopedRWLockT {
173-
174-
ScopedRWLockT() = delete;
175-
ScopedRWLockT(const ScopedRWLockT &) = delete;
176-
ScopedRWLockT &operator=(const ScopedRWLockT &) = delete;
177-
ScopedRWLockT(ScopedRWLockT &&) = delete;
178-
ScopedRWLockT &operator=(ScopedRWLockT &&) = delete;
179-
180-
public:
181-
explicit ScopedRWLockT(T &l) : Lock(l) {
182-
if (Inverted) {
183-
if (Read) {
184-
Lock.readUnlock();
185-
} else {
186-
Lock.writeUnlock();
187-
}
188-
} else {
189-
if (Read) {
190-
Lock.readLock();
191-
} else {
192-
Lock.writeLock();
193-
}
194-
}
195-
}
196-
197-
~ScopedRWLockT() {
198-
if (Inverted) {
199-
if (Read) {
200-
Lock.readLock();
201-
} else {
202-
Lock.writeLock();
203-
}
204-
} else {
205-
if (Read) {
206-
Lock.readUnlock();
207-
} else {
208-
Lock.writeUnlock();
209-
}
210-
}
211-
}
212-
213-
private:
214-
T &Lock;
215-
};
216-
217-
class ReadWriteLock;
218-
class StaticReadWriteLock;
219-
220-
/// A stack based object that unlocks the supplied ReadWriteLock on
221-
/// construction and locks it for reading on destruction.
222-
///
223-
/// Precondition: ReadWriteLock unlocked by this thread, undefined otherwise.
224-
typedef ScopedRWLockT<ReadWriteLock, true, false> ScopedReadLock;
225-
typedef ScopedRWLockT<StaticReadWriteLock, true, false> StaticScopedReadLock;
226-
227-
/// A stack based object that unlocks the supplied ReadWriteLock on
228-
/// construction and locks it for reading on destruction.
229-
///
230-
/// Precondition: ReadWriteLock unlocked by this thread, undefined
231-
/// otherwise.
232-
typedef ScopedRWLockT<ReadWriteLock, true, true> ScopedReadUnlock;
233-
typedef ScopedRWLockT<StaticReadWriteLock, true, true> StaticScopedReadUnlock;
234-
235-
/// A stack based object that unlocks the supplied ReadWriteLock on
236-
/// construction and locks it for reading on destruction.
237-
///
238-
/// Precondition: ReadWriteLock unlocked by this thread, undefined otherwise.
239-
typedef ScopedRWLockT<ReadWriteLock, false, false> ScopedWriteLock;
240-
typedef ScopedRWLockT<StaticReadWriteLock, false, false> StaticScopedWriteLock;
241-
242-
/// A stack based object that unlocks the supplied ReadWriteLock on
243-
/// construction and locks it for writing on destruction.
244-
///
245-
/// Precondition: ReadWriteLock unlocked by this thread, undefined otherwise.
246-
typedef ScopedRWLockT<ReadWriteLock, false, true> ScopedWriteUnlock;
247-
typedef ScopedRWLockT<StaticReadWriteLock, false, true> StaticScopedWriteUnlock;
248-
249-
/// A Read / Write lock object that has semantics similar to `BasicLockable`
250-
/// and `Lockable` C++ concepts however it supports multiple concurrent
251-
/// threads holding the reader lock as long as the write lock isn't held and
252-
/// only one thread can hold the write local at the same time.
253-
///
254-
/// If you need static allocated ReadWriteLock use StaticReadWriteLock.
255-
///
256-
/// See http://en.cppreference.com/w/cpp/concept/BasicLockable
257-
/// See http://en.cppreference.com/w/cpp/concept/Lockable
258-
///
259-
/// This is NOT a recursive mutex.
260-
class ReadWriteLock {
261-
262-
ReadWriteLock(const ReadWriteLock &) = delete;
263-
ReadWriteLock &operator=(const ReadWriteLock &) = delete;
264-
ReadWriteLock(ReadWriteLock &&) = delete;
265-
ReadWriteLock &operator=(ReadWriteLock &&) = delete;
266-
267-
public:
268-
ReadWriteLock() { ReadWriteLockPlatformHelper::init(Handle); }
269-
~ReadWriteLock() { ReadWriteLockPlatformHelper::destroy(Handle); }
270-
271-
/// The readLock() method has the following properties:
272-
/// - Behaves as an atomic operation.
273-
/// - Blocks the calling thread while the write lock is held by another
274-
/// thread and once the read lock is acquired by the calling thread
275-
/// other threads are prevented from acquiring the write lock.
276-
/// - Multiple threads can hold the read lock at the same time.
277-
/// - Prior unlock() operations on the same lock synchronize-with
278-
/// this lock operation.
279-
/// - The behavior is undefined if the calling thread already owns
280-
/// the read or write lock (likely a deadlock).
281-
/// - Does not throw exceptions but will halt on error (fatalError).
282-
///
283-
/// Callers must not mutate the data protected by the ReadWriteLock while
284-
/// holding the read lock, the write lock must be used.
285-
void readLock() { ReadWriteLockPlatformHelper::readLock(Handle); }
286-
287-
/// The try_readLock() method has the following properties:
288-
/// - Behaves as an atomic operation.
289-
/// - Attempts to obtain the read lock without blocking the calling thread.
290-
/// If ownership is not obtained, returns immediately. The function is
291-
/// allowed to spuriously fail and return even if the lock is not
292-
/// currently owned by another thread.
293-
/// - If try_readLock() succeeds, prior unlock() operations on the same
294-
/// object synchronize-with this operation. unlock() does not synchronize
295-
/// with a failed try_readLock().
296-
/// - The behavior is undefined if the calling thread already owns
297-
/// the read or write lock (likely a deadlock)?
298-
/// - Does not throw exceptions but will halt on error (fatalError).
299-
///
300-
/// Callers must not mutate the data protected by the ReadWriteLock while
301-
/// holding the read lock, the write lock must be used.
302-
bool try_readLock() {
303-
return ReadWriteLockPlatformHelper::try_readLock(Handle);
304-
}
305-
306-
/// The readUnlock() method has the following properties:
307-
/// - Behaves as an atomic operation.
308-
/// - Releases the calling thread's ownership of the read lock
309-
/// and synchronizes-with the subsequent successful lock operations on
310-
/// the same object.
311-
/// - The behavior is undefined if the calling thread does not own
312-
/// the read lock.
313-
/// - Does not throw exceptions but will halt on error (fatalError).
314-
void readUnlock() { ReadWriteLockPlatformHelper::readUnlock(Handle); }
315-
316-
/// The writeLock() method has the following properties:
317-
/// - Behaves as an atomic operation.
318-
/// - Blocks the calling thread while the write lock or a read lock is held
319-
/// by another thread and once the write lock is acquired by the calling
320-
/// thread other threads are prevented from acquiring the write lock or a
321-
/// read lock.
322-
/// - Only one thread can hold the write lock at the same time.
323-
/// - Prior unlock() operations on the same lock synchronize-with
324-
/// this lock operation.
325-
/// - The behavior is undefined if the calling thread already owns
326-
/// the read or write lock (likely a deadlock).
327-
/// - Does not throw exceptions but will halt on error (fatalError).
328-
void writeLock() { ReadWriteLockPlatformHelper::writeLock(Handle); }
329-
330-
/// The try_writeLock() method has the following properties:
331-
/// - Behaves as an atomic operation.
332-
/// - Attempts to obtain the write lock without blocking the calling thread.
333-
/// If ownership is not obtained, returns immediately. The function is
334-
/// allowed to spuriously fail and return even if the lock is not
335-
/// currently owned by another thread.
336-
/// - If try_writeLock() succeeds, prior unlock() operations on the same
337-
/// object synchronize-with this operation. unlock() does not synchronize
338-
/// with a failed try_writeLock().
339-
/// - The behavior is undefined if the calling thread already owns
340-
/// the read or write lock (likely a deadlock)?
341-
/// - Does not throw exceptions but will halt on error (fatalError).
342-
bool try_writeLock() {
343-
return ReadWriteLockPlatformHelper::try_writeLock(Handle);
344-
}
345-
346-
/// The writeUnlock() method has the following properties:
347-
/// - Behaves as an atomic operation.
348-
/// - Releases the calling thread's ownership of the write lock
349-
/// and synchronizes-with the subsequent successful lock operations on
350-
/// the same object.
351-
/// - The behavior is undefined if the calling thread does not own
352-
/// the write lock.
353-
/// - Does not throw exceptions but will halt on error (fatalError).
354-
void writeUnlock() { ReadWriteLockPlatformHelper::writeUnlock(Handle); }
355-
356-
/// Acquires read lock before calling the supplied critical section and
357-
/// releases lock on return from critical section. Callers must not mutate
358-
/// the data protected by the ReadWriteLock while holding the read lock, the
359-
/// write lock must be used.
360-
///
361-
/// This call can block while waiting for the lock to become available.
362-
///
363-
/// For example the following reads the cached value while holding
364-
/// the read lock.
365-
///
366-
/// ```
367-
/// rw.withReadLock([&value] { value = cachedValue; });
368-
/// ```
369-
///
370-
/// Precondition: ReadWriteLock not held by this thread, undefined otherwise.
371-
template <typename CriticalSection>
372-
auto withReadLock(CriticalSection &&criticalSection)
373-
-> decltype(std::forward<CriticalSection>(criticalSection)()) {
374-
ScopedReadLock guard(*this);
375-
return std::forward<CriticalSection>(criticalSection)();
376-
}
377-
378-
/// Acquires write lock before calling the supplied critical section and
379-
/// releases lock on return from critical section.
380-
///
381-
/// This call can block while waiting for the lock to become available.
382-
///
383-
/// For example the following updates the cached value while holding
384-
/// the write lock.
385-
///
386-
/// ```
387-
/// rw.withWriteLock([&newValue] { cachedValue = newValue });
388-
/// ```
389-
///
390-
/// Precondition: ReadWriteLock not held by this thread, undefined otherwise.
391-
template <typename CriticalSection>
392-
auto withWriteLock(CriticalSection &&criticalSection)
393-
-> decltype(std::forward<CriticalSection>(criticalSection)()) {
394-
ScopedWriteLock guard(*this);
395-
return std::forward<CriticalSection>(criticalSection)();
396-
}
397-
398-
private:
399-
ReadWriteLockHandle Handle;
400-
};
401-
402174
/// A static allocation variant of Mutex.
403175
///
404176
/// Use Mutex instead unless you need static allocation.
@@ -450,66 +222,6 @@ class StaticMutex {
450222
MutexHandle Handle;
451223
};
452224

453-
/// A static allocation variant of ReadWriteLock.
454-
///
455-
/// Use ReadWriteLock instead unless you need static allocation.
456-
class StaticReadWriteLock {
457-
458-
StaticReadWriteLock(const StaticReadWriteLock &) = delete;
459-
StaticReadWriteLock &operator=(const StaticReadWriteLock &) = delete;
460-
StaticReadWriteLock(StaticReadWriteLock &&) = delete;
461-
StaticReadWriteLock &operator=(StaticReadWriteLock &&) = delete;
462-
463-
public:
464-
#if SWIFT_READWRITELOCK_SUPPORTS_CONSTEXPR
465-
constexpr
466-
#endif
467-
StaticReadWriteLock()
468-
: Handle(ReadWriteLockPlatformHelper::staticInit()) {
469-
}
470-
471-
/// See ReadWriteLock::readLock
472-
void readLock() { ReadWriteLockPlatformHelper::readLock(Handle); }
473-
474-
/// See ReadWriteLock::try_readLock
475-
bool try_readLock() {
476-
return ReadWriteLockPlatformHelper::try_readLock(Handle);
477-
}
478-
479-
/// See ReadWriteLock::readUnlock
480-
void readUnlock() { ReadWriteLockPlatformHelper::readUnlock(Handle); }
481-
482-
/// See ReadWriteLock::writeLock
483-
void writeLock() { ReadWriteLockPlatformHelper::writeLock(Handle); }
484-
485-
/// See ReadWriteLock::try_writeLock
486-
bool try_writeLock() {
487-
return ReadWriteLockPlatformHelper::try_writeLock(Handle);
488-
}
489-
490-
/// See ReadWriteLock::writeUnlock
491-
void writeUnlock() { ReadWriteLockPlatformHelper::writeUnlock(Handle); }
492-
493-
/// See ReadWriteLock::withReadLock
494-
template <typename CriticalSection>
495-
auto withReadLock(CriticalSection &&criticalSection)
496-
-> decltype(std::forward<CriticalSection>(criticalSection)()) {
497-
StaticScopedReadLock guard(*this);
498-
return std::forward<CriticalSection>(criticalSection)();
499-
}
500-
501-
/// See ReadWriteLock::withWriteLock
502-
template <typename CriticalSection>
503-
auto withWriteLock(CriticalSection &&criticalSection)
504-
-> decltype(std::forward<CriticalSection>(criticalSection)()) {
505-
StaticScopedWriteLock guard(*this);
506-
return std::forward<CriticalSection>(criticalSection)();
507-
}
508-
509-
private:
510-
ReadWriteLockHandle Handle;
511-
};
512-
513225
/// A Mutex object that supports `BasicLockable` C++ concepts. It is
514226
/// considered
515227
/// unsafe to use because it doesn't do any error checking. It is only for
@@ -634,13 +346,6 @@ static_assert(std::is_literal_type<StaticConditionVariable>::value,
634346
// you will possibly see global-constructors warnings
635347
#endif
636348

637-
#if SWIFT_READWRITELOCK_SUPPORTS_CONSTEXPR
638-
static_assert(std::is_literal_type<StaticReadWriteLock>::value,
639-
"StaticReadWriteLock must be literal type");
640-
#else
641-
// Your platform doesn't currently support statically allocated ReadWriteLocks
642-
// you will possibly see global-constructors warnings
643-
#endif
644349
}
645350

646351
#endif

0 commit comments

Comments
 (0)