Skip to content

Commit 7372e7e

Browse files
committed
Make it harder to forget to update an existing AtomicWaitQueue.
The existing uses of AWQ don't need arguments during construction, but uses that do almost certainly need to update existing instances if createQueue happens to re-use one. Users probably aren't going to think about this proactively by doing something wild like reading the documentation. We can point this mistake out to them by making their code not compile if they call createQueue with arguments without providing a special method. This pattern also makes the actual update code much easier to write, since callers don't need to specially detect this case.
1 parent 80af928 commit 7372e7e

File tree

1 file changed

+18
-3
lines changed

1 file changed

+18
-3
lines changed

include/swift/Runtime/AtomicWaitQueue.h

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,17 @@ class AtomicWaitQueue {
106106
return referenceCount == 1;
107107
}
108108

109+
/// This queue is being re-used with new construction arguments.
110+
/// Update it appropriately.
111+
void updateForNewArguments() {
112+
// We intentionally take no arguments so that only calls to
113+
// createQueue with no arguments will succeed at calling this no-op
114+
// implementation. Queue types with construction arguments
115+
// will need to implement this method to take the appropriate
116+
// arguments. Hopefully this discourages people from forgetting
117+
// that queues can be re-used if created in a loop.
118+
}
119+
109120
/// An RAII helper class for signalling that the current thread is a
110121
/// worker thread which has acquired the lock.
111122
///
@@ -195,14 +206,18 @@ class AtomicWaitQueue {
195206
///
196207
/// The Worker object takes ownership of the queue until it's
197208
/// published, so you can safely call this even if publishing
198-
/// might fail. Note that the same queue will be returned on
199-
/// successive invocations, so take care if the arguments might
200-
/// change during the loop.
209+
/// might fail.
210+
///
211+
/// Note that the same queue will be returned on successive
212+
/// invocations. Queues that accept arguments for construction
213+
/// should implement `updateForNewArguments`.
201214
template <class... Args>
202215
Impl *createQueue(Args &&...args) {
203216
assert(!Published);
204217
if (!CurrentQueue)
205218
CurrentQueue = asImpl().createNewQueue(std::forward<Args>(args)...);
219+
else
220+
CurrentQueue->updateForNewArguments(std::forward<Args>(args)...);
206221
return CurrentQueue;
207222
}
208223

0 commit comments

Comments
 (0)