NullReference at seemingly innocent WeakReference access?

Question

So, I have a piece of code using WeakReferences. I know of the common .IsAlive race condition, so I didn't use that. I basically have something like this:

WeakReference lastString=new WeakReference(null);
public override string ToString()
{
  if(lastString!=null)
  {
    var s=lastString.Target as string; //error here
    if(s!=null)
    {
      return s;
    }
  }
  var str=base.ToString();
  lastString=new WeakReference(str);
  return str;
}

Somehow, I'm getting a null reference exception at the marked line. By debugging it, I can confirm that lastString is indeed null, despite being wrapped in a null check and lastString never actually being set to null.

This only happens in a complex flow as well, which makes me think garbage collection is somehow taking my actual WeakReference object, and not just it's target.

Can someone enlighten me as to how this is happening and what the best course of action is?

EDIT: I can't determine at all the cause of this. I ended up wrapping the error code in a try-catch just fix it for now. I'm very interested in the root cause of this though. I've been trying to reproduce this in a simple test case, but it's proven very difficult to do. Also, this only appears to happen when running under a unit test runner. If I take the code and trim it down to the minimum, it will continue to crash when running using TestDriven and Gallio, but will not fail when put into a console application

@Tigran I can't reproduce this in a standalone simple test case. The program it is in is multithreaded, but this particular object should never be access by more than one thread at once (which I can confirm with reasonable certainty with a debugger) — Earlz, Jul 02 '13 at 20:01
Does this happen every time you execute the "complex flow", or just occationally? — Cemafor, Jul 02 '13 at 20:04
Also, even if it was multithreaded, `lastString` is still never null, and assignment is atomic, so this should be thread safe, assuming that base.ToString is thread safe — Earlz, Jul 02 '13 at 20:05
@Cemafor every time. I have a unit test that's exposed it, but can't successfully reproduce it outside of our closed code. Also, this happens in Release and Debug modes — Earlz, Jul 02 '13 at 20:06

score 1 · Answer 1 · answered Jul 03 '13 at 14:26

This ended up being a very hard to spot logic bug that was in plain sight.

The offending if statement really was more like this:

if(lastString!=null && limiter==null || limiter=lastLimiter)

The true grouping of this is more like this:

if((lastString!=null && limiter==null) || limiter=lastLimiter)

And as Murphy's law would dictate, somehow, in this one unrelated test case, lastLimiterand lastString got set to null by a method used no where but this one single test case.

So yea, no bug in the CLR, just my own logic bug that was very hard to spot

NullReference at seemingly innocent WeakReference access?

1 Answers1