C#: Properly using WeakReference IsAlive property

Question

As explained here, if WeakReference's IsAlive returns true, then it cannot be trusted. Now, I'm trying to understand the correct way to use this:

Incorrect:

WeakReference dogRef = new WeakReference(dog);

// Later, try to ref original Dog

if (dogRef.IsAlive)
{
    // Oops - garbage collection on original Dog could occur here
    ((Dog)dogRef.Target).Bark();
}

Correct:

WeakReference dogRef = new WeakReference(dog);

// Later, try to ref original Dog

Dog origDog = (Dog)dogRef.Target;
if (origDog != null)
{
    origDog.Bark();
}

My question is, is there any difference from the point of view of the GC, between if(origDog != null and dogRef.Target != null? Suppose, I did not need to invoke methods of the Dog class but just needed to check if the target was alive. Should I always cast the target to a class instance or is it okay to check Target against null?

I ask this because I want to execute some logic if the object is alive that does not involve the object itself.

Answer 1

My question is, is there any difference from the point of view of the GC, between if(origDog != null and dogRef.Target != null?

With origDog then if origDog is not null (because there was indeed a reference when dogRef.Target was assigned to it, then it will continue to not be null until it is either written over, or becomes collectable.

With dogRef.Target != null then problem isn't that call — it will work fine — but with the time in between that and the attempt to use it.

(Incidentally, while it's more typing, as a rule creating a temporary value on the stack instead of hitting a property twice is generally going to be slightly more efficient. It's not so much more efficient that to make it worth doing when calling the property is more natural to type, but it is worth noting if the only reason for not wanting to create the temporary dogRef was fear that it was extra work for the application).

From a comment:

If comparing Target with null obtains a strong reference until the end of the scope

It doesn't, and nor does assigning. It's important to realise that scope has nothing to do with collectability. In the code:

void SomeMethodWhichThereforeHasAScope()
{
  Dog origDog = (Dog)dogRef.Target;
  if (origDog != null)
  {
    Console.Write(dogRef.Target == null); // probably going to be false (though sometimes reads get reordered, so there's a chance that happens).
    origDog.Bark();
  }

  Console.Write(dogRef.Target == null); // could be true or false
  var sb = new StringBuilder("I'm a string that got referenced in a call to a method");
  Console.Write(sb.ToString());
  Console.Write(dogRef.Target == null); // even more likely to be true.
}

origDog is in scope at the time of the third test of Target but it isn't used after it. Which means the reference to the object on the stack and/or in a register that was used to invoke Bark may have been used for something else (more likely the more work has happened in the method), which means that if the GC kicks in it may not find a reference.

"Scope" is about where you can use a variable. The GC works based on where you did use it. Once you've stopped using it, the GC might reclaim objects it referenced. Normally we don't care, because we don't use something after we use it (ipso facto) so we won't notice. Having another reference that goes through WeakReference changes that though.

This is why GC.KeepAlive() exists. It doesn't actually do anything, it's just a method that won't be optimised away so that if the only reason you want to keep a variable in scope is that some unusual GC stuff (WeakReference fits into the category of "unusual GC stuff") means you might want to use the same object through something other than that variable, it won't be collected until after that KeepAlive() call.

Suppose, I did not need to invoke methods of the Dog class but just needed to check if the target was alive. Should I always cast the target to a class instance or is it okay to check Target against null?

Checking it's not null is fine. Indeed using IsAlive is fine. The problem with IsAlive is purely that it may become false at some point in the future. This is true for any means of checking on life.

(The only time I ever saw Luciano Pavarotti he was alive. I have not seen him since. The fact that he was perfectly alive the last time I saw him does not stop him being dead now. WeakReference.IsAlive is exactly the same).

Incidentally, for single calls the following is valid and convenient:

((Dog)dogRef.Target)?.Bark();

Because the ?. operator will dup the reference so it's akin to:

var temp = ((Dog)dogRef.Target)
if (temp != null)
  temp.Bark();

So it's safe.

If there is a disconnect between the object and the work you will do you can use:

var temp = dogRef.Target;
if (temp != null)
{
   DoStuffHere();
   GC.KeepAlive(temp); // temp cannot be collected until this returns.
}

As said above, KeepAlive() is just a no-operation method that the compiler and jitter are not allowed to optimise away. As such there has to be a reference on the stack or in a register to pass to it, and the GC will see that and not collect it.

Answer 2

The purpose of IsAlive is not to let code know when it's safe to use a WeakReference, but rather to check whether a reference has died off yet without any possibility of keeping it alive needlessly.

If code wants to use the target of a WeakReference, it must copy that to a variable and then check whether it's null; if the assignment occurs before the reference dies, the copy of the target stored in the variable will keep the reference alive. The danger with using if (weakref.Target == null) handleObjectDeath(); is that if no reference other reference exists to the target but the above code occurs just as a GC cycle is about to start, the GC might hold off on collecting the object (and invalidating the weak reference) because of the temporary reference returned by the Target property. Using the IsAlive property avoids that risk.