> On Dec 10, 2015, at 10:55 PM, John McCall <rjmcc...@apple.com> wrote: > > The intent is that weak references do not need to be safe against read/write > and write/write races. They do need to be safe against read/destroy and > write/destroy races (by “destroy”, I mean destruction of the object, not the > weak reference). I agree that they should also be safe against read/read > races. > >> The root of the problem (or not-problem, if it's supposed to be this way) is >> in the swift_weakLoadStrong function in HeapObject.cpp. If two threads >> simultaneously enter this function with the same weak reference and >> object->refCount.isDeallocating() is true, the threads will race. This can >> result in calling swift_unownedRelease() twice resulting in a double-free or >> worse, or it could result in one thread calling swift_tryRetain() on a >> deallocated object. > > Yes, you’re absolutely right, this is a bug in the current implementation; > good catch!
OK! I filed the bug: https://bugs.swift.org/browse/SR-192 > There’s actually a higher-level semantic bug in this code, which is that we > really do want weak references to make the same deallocation guarantees as > Objective-C if possible. That is, while we’re comfortable with the idea of > an unowned reference pinning some memory until the unowned reference is > destroyed, weak references should really allow the object’s memory to be > more-or-less immediately returned to the general pool. It was always our > intention to revisit the implementation and do this. > > It would be acceptable to increase the inline size of weak references if that > makes this more performant. It's interesting that you say that. I discovered this bug while writing up the weak reference implementation for a blog article. (Which I just posted here: https://mikeash.com/pyblog/friday-qa-2015-12-11-swift-weak-references.html) My conclusion is that the Swift Way of leaving an object husk lying around in memory for a while is superior to the Objective-C approach of eagerly deallocating objects and zeroing out references. The memory impact is small, and the performance improvement is nice. Zeroing weak references are always good to have but the cost of accessing them in Objective-C always bugged me a bit. (And I say this having written my own implementation that works exactly the same way.) My own thinking is that object instances are not usually very big. As long as you can destroy the external resources it holds such as arrays and dictionaries and trees of other objects, the cost is small, just one allocation of some dozens of bytes. You have at most one such allocation per weak reference to a dead object, and less than that if there are multiple weak references to the same dead object. (I hope I am understanding the implementation correctly that it does in fact tear down the object completely when the last strong reference is released, and just keeps the instance's own memory around for the weak references.) Increasing the inline size of weak references opens up the possibilities a bit. I can think of at least four fixes: 1. Delete the zeroing, and otherwise leave things as-is. This extends the life of the object husk (by the way, is there an official term?) possibly indefinitely. This seems fine to me but it sounds like you may disagree. I will, of course, defer to your judgment on that. 2. Add an activity count to the weak reference. WeakReference would become something like struct WeakReference { HeapObject *Value; unsigned long count; }. swift_weakLoadStrong would increment the count when loading the pointer, and decrement the count when done. Zeroing would only happen when decrementing the count to zero. This would require a 16-byte compare-and-swap operation. 3. Borrow a bit from the weak pointer to implement a spinlock. This is really a special case of (2), with the activity count being capped at 1 and additional activity blocking. In fact, you could even do a hybrid approach by borrowing more bits. (I think it could safely steal up to 20 bits with current 64-bit architectures. This may not be wise. As long as targets are pointer-aligned you can safely steal 2/3 bits.) 4. Add additional bookkeeping and synchronization to allow eager zeroing and deallocation of object husks more like the Objective-C weak pointer implementation. This would require a list of weak references to a given object to be maintained somewhere. I personally would rank my preference as 1, 3, 2, 4. You guys have *slightly* more experience with this code than I do, though, so I put little weight on my own preference here. What do you think? Mike _______________________________________________ swift-dev mailing list swift-dev@swift.org https://lists.swift.org/mailman/listinfo/swift-dev
