Purpose of std::atomic in WeakReference

Hello! I’m exploring the runtime source code now, and I can’t figure out why std::atomic is needed in WeakReferenceclass. The comment at the top of the file states:

// Thread-safety:
// 
// Reading a weak reference must be thread-safe with respect to:
//  * concurrent readers
//  * concurrent weak reference zeroing due to deallocation of the
//    pointed-to object
//  * concurrent ObjC readers or zeroing (for non-native weak storage)
// 
// Reading a weak reference is NOT thread-safe with respect to:
//  * concurrent writes to the weak variable other than zeroing
//  * concurrent destruction of the weak variable
//
// Writing a weak reference must be thread-safe with respect to:
//  * concurrent weak reference zeroing due to deallocation of the
//    pointed-to object
//  * concurrent ObjC zeroing (for non-native weak storage)
//
// Writing a weak reference is NOT thread-safe with respect to:
//  * concurrent reads
//  * concurrent writes other than zeroing

Here's my understanding:
Essentially thread-safe access should be preserved for reading and reading, reading and zeroing, writing and zeroing (I'm omitting the ObjC case here, because nonnativeValue is not wrapped in atomic at all). But zeroing of reference due to its deallocation does not seem to affect WeakReferenceBits in any way. It just deallocates an object, putting it into the "Freed" state, but side table remains alive, and nativeValue bits point to the same object, so it doesn't seem like this can introduce any races on this variable. Concurrent reads are usually safe on their own.

I searched through the class implementation for store calls, and found them in two groups of methods: nativeCopyInitFromBits, nativeInit, nativeDestroy and nativeAssign, which all seem to be write methods, so should not require synchronization. nativeTakeStrong, nativeTakeInit and unknownTakeStrong have pretty straightforward implementations, but I wasn't able to come up with some code snippet which emits any of them.

So, my question is: what I am misunderstanding? Is synchronization of nativeValue actually needed for safe deallocation, or for the -Take- calls? How can I make the compiler emit the -Take- calls?

Thanks in advance.

This implementation was a bit before my time, but looking through it, I think the ObjC case is the key. nonnativeValue is never really touched, we just pass the address to/from the ObjC runtime which will modify it (presumably atomically). And because it’s a union, writes to one version will be visible in the other.

ObjC weak references work very differently. The ObjC runtime eagerly zeroes all registered weak references when the target object is destroyed. So after the Swift runtime has called objc_initWeak, at some point later on the ObjC runtime may zero the value.

If the ObjC runtime concurrently reads the value while that happens, it all works out. ObjC handles this so that you either load the object or nil. No problem there.

But before the ObjC runtime will read this value, the Swift runtime reads it! In unknownLoadStrong and unknownTakeStrong, we load nativeValue and use its value to determine which kind of reference we have.

If this load was not atomic, then we’d be vulnerable to tearing in this scenario. If the Swift runtime did a read while the ObjC runtime was zeroing the value, such that the Swift runtime got a value that was a mix of old and new, then we could end up in trouble. We might see the lower part of the value zeroed, which would put us in the isNativeOrNull case. But if the high part is still the old value, then we’d see it as a non-null native reference with some bogus value, and most likely crash.

This is very unlikely to be a problem in practice, since loading a pointer value is likely to compile down to a single load instruction which will be atomic on any supported target, but it is technically needed for correctness.

5 Likes