10

As a seasoned C++ programmer trying to get accustomed to .NET, there's an implementation detail in Microsoft's WeakReference "Target" property that's bugging me...

public class WeakReference : ISerializable
{
    internal IntPtr m_handle;
    internal bool m_IsLongReference;
                 ...
    public virtual object Target
    {
        [SecuritySafeCritical]
        get
        {
            IntPtr handle = this.m_handle;
            if (IntPtr.Zero == handle)
            {
                return null;
            }
            object result = GCHandle.InternalGet(handle);
            if (!(this.m_handle == IntPtr.Zero))
            {
                return result;
            }
            return null;
        }
        [SecuritySafeCritical]
        set
        {
            IntPtr handle = this.m_handle;
            if (handle == IntPtr.Zero)
            {
                throw new InvalidOperationException(Environment.GetResourceString("InvalidOperation_HandleIsNotInitialized"));
            }
            object oldValue = GCHandle.InternalGet(handle);
            handle = this.m_handle;
            if (handle == IntPtr.Zero)
            {
                throw new InvalidOperationException(Environment.GetResourceString("InvalidOperation_HandleIsNotInitialized"));
            }
            GCHandle.InternalCompareExchange(handle, value, oldValue, false);
            GC.KeepAlive(this);
        }
    }
     ...
       }

The thing that's bugging me is this - why are they checking the validity of m_handle twice? Particularly in the 'set' method - the use of the GC.KeepAlive at the end of the method should keep the WeakReference from being garbage collected, and thus keep the handle non-zero - right?

And in the case of the 'get' - once we've actually retrieved a reference to the target via InternalGet, why bother checking the original m_handle value again? All I can think is that perhaps they're trying to guard against the WeakReference being disposed and finalized either during or after the InternalGet - but surely, couldn't it also be disposed and finalized before we get around to returning the object? I just can't come up w/ a valid explanation as to why this double-checking is necessary here...

Kevin
  • 1,120
  • 6
  • 13
  • Why do you care about the implementation that much? And is this actual Microsoft licensed code you are posting, or is it Mono's? – Dykam Dec 29 '11 at 19:19
  • +1 Good question. Doesn't seem to make much sense in either of the accessors. – vgru Dec 29 '11 at 19:20
  • 7
    @Dykam Reading code is an excellent way to learn, and treating library code as "magic" is an excellent way not to learn and to develop superstitions. Also, I would hate to think that this snippet does not fall under fair use even if it's copyrighted. – Pascal Cuoq Dec 29 '11 at 19:26
  • @Complicatedseebio, that is very true, but I would be a bit hesitant to post MS code myself. – Dykam Dec 29 '11 at 20:33

1 Answers1

8

All I can think is that perhaps they're trying to guard against the WeakReference being disposed and finalized either during or after the InternalGet

That's exactly right.

but surely, couldn't it also be disposed and finalized before we get around to returning the object?

No, because at that point, a strong pointer must have been created to the object. InternalGet returns a strong pointer, and if that strong pointer, stored in oldValue, is to the object, now the object can no longer be reclaimed by the garbage collector.

Pascal Cuoq
  • 79,187
  • 7
  • 161
  • 281
  • The call to `InternalGet` dosn't create strong pointer itself, but whatever implementation it has. – Tomislav Markovski Dec 29 '11 at 19:23
  • @TomislavMarkovski It effectively does, as it's creating a root to the object referenced in `GCHandle` (the return value). Once you store the return value (oldValue), you've got a rooted GC reference, so the object is no longer eligible for GC. – Reed Copsey Dec 29 '11 at 19:29
  • Thanks! Still - I don't quite see the logic. The only thing that can zero out the internal handle is the finalizer - so the only way that handle can change during this call is if the WeakReference's finalizer is called along the way. Let's say I have one of those "zombie" WeakReference's to this WeakReference - so I call "get" on this WeakReference after its finalizer has been scheduled. Still - the only way that second check on the handle can make a difference is if the WR's finalizer is called somewhere between the first and second check. – Kevin Jan 15 '12 at 22:32
  • But - couldn't the finalizer also be called between the second check and the return? I guess what I'm seeing is that anything that depends on the operation of that second check is still a race condition - even with the second check... – Kevin Jan 15 '12 at 22:33
  • @Kevin I know about weak pointers, but not much about .NET's particulars. Still, I think you have it backwards when you say "the only thing that can zero out the internal handle is the finalizer". The Garbage Collector zeroes out things (and calls finalizers when appropriate). And the garbage collector is designed never to do that for an object to which there exists a live strong reference, which `oldValue` is in the sequence of events that preoccupies you. – Pascal Cuoq Jan 16 '12 at 09:31
  • @PascalCuoq: The way finalization is designed on .NET, it is possible for the finalizer of one object to resurrect other objects whose finalizers have been scheduled but have not yet run. The fact that a strong rooted reference presently exists to an object does not guarantee that its finalizer didn't get scheduled at a time when no such reference existed. – supercat Sep 19 '13 at 16:15