0

I had a unit test that had been passing for months and months (years?) suddenly start failing on me last week in debug builds only (Visual Studio 2017 15.7.5, .net framework 4.5). It relies on an object referenced by a local variable becoming garbage after that variable was set to null. I was able to distill things down to the below (no test framework required):

private class Foo
{
    public static int Count;
    public Foo() => ++Count;
    ~Foo() => --Count;
}

public void WillFail()
{
    var foo = new Foo();
    Debug.Assert(Foo.Count == 1);
    foo = null;
    GC.Collect();
    GC.WaitForPendingFinalizers();
    Debug.Assert(Foo.Count == 0);
}

Setting a breakpoint on the second assert and taking a memory snapshot shows that there is indeed one Foo object in memory, and its root is "local variable." Enclosing the first three lines in their own set of {}s doesn't make any difference, but extracting them into a local function allows the test to pass:

public void WillPass()
{
    DoIt();
    GC.Collect();
    GC.WaitForPendingFinalizers();
    Debug.Assert(Foo.Count == 0);

    void DoIt()
    {
        var foo = new Foo();
        Debug.Assert(Foo.Count == 1);
    }
}

What gives? I was under the impression that objects became garbage the moment the last reference to them went away--more than that; I've been warned by several authors that objects can become garbage while there are still references to them in scope, as long as those references aren't used again. And the fact that this test used to work suggests I was right. But now it looks like an object referenced by a local variable (at least) isn't garbage until the end of the function that contains the variable. Did something change?

dlf
  • 9,045
  • 4
  • 32
  • 58
  • Use `Interlocked.Increment()` and `Decrement()` instead of ++ and --. The destructor is called from a different (GC finalizer-)thread. – Tim Schmelter Jul 30 '18 at 14:00
  • @TimSchmelter Same result. But I found out the original test does pass in a release build, just not debug... – dlf Jul 30 '18 at 14:04
  • 1
    If you have unit tests that are trying to verify that *the garbage collector is working "correctly"* (based on *your* interpretation of how the GC "should" work), and you're not working on the team that looks after the garbage collector, perhaps ask yourself if your tests are correctly focussed? `GC.Collect` doesn't really have much more place in unit tests than it does in production code. – Damien_The_Unbeliever Jul 30 '18 at 14:15
  • @Damien_The_Unbeliever The real test is verifying that object A holds nothing but weak references to object B. We do this by confirming that a B becomes garbage after nothing but an A knows about it anymore. – dlf Jul 30 '18 at 14:19
  • 2
    Well, just bear in mind that many details of the GC are *implementation* details. Who knows, in the future your code could find itself running in an environment with a [Zero (a.k.a. Null) Garbage Collector](http://tooslowexception.com/zero-garbage-collector-for-net-core/). This the simplest and most extreme example, and unlikely to ever be the *default* (for a desktop CLR) - but it's a perfectly valid GC for a program that doesn't need the GC to [simulate infinite memory](https://blogs.msdn.microsoft.com/oldnewthing/20100809-00/?p=13203) – Damien_The_Unbeliever Jul 30 '18 at 14:29

1 Answers1

1

I had a unit test that had been passing for months and months (years?) suddenly start failing on me last week (Visual Studio 2017 15.7.5, .net framework 4.5). It relied on an object referenced by a local variable becoming garbage after that variable was set to null.

You should never rely on that in production code because, regardless of whether your particular test passes or not, the .NET GC is working exactly as specified.

Setting a breakpoint on the second assert and taking a memory snapshot shows that there is indeed one Foo object in memory, and its root is "local variable." Enclosing the first three lines in their own set of {}s doesn't make any difference, but extracting them into a local function allows the test to pass.

The inner scope you created is not reflected in the CIL code, that's why it makes no difference. The local function, on the other hand, will probably have its stack frame cleaned up when it returns (unless other mechanisms kick in to cancel that effect as well).

I was under the impression that objects became garbage the moment the last reference to them went away

Objects without any reachable reference left are eligible for garbage collection, but when they will actually be collected is a decision made by the GC. This is especially true for objects with finalizers, which are put in a finalization queue before being reclaimed.

I've been warned by several authors that objects can become garbage while there are still references to them in scope, as long as those references aren't used again.

This happens when the compiler can prove that the remaining references to the object will not be accessed again, even if they are in scope in the source code. The runtime mechanisms that decide this do not have a concept of the source code anyway. Ephemeral variables (which are created and then used just once) are prime candidates for this optimization.

However, if you are building and running your program under a Debug configuration, then the compiler and the runtime will refrain from making the aforementioned optimization, because it would hinder debugging (which is the entire point of the build), by deleting the values of variables that could still be inspected by you when the program is in Break mode.

Theodoros Chatzigiannakis
  • 28,773
  • 8
  • 68
  • 104
  • _"...collected is a decision made by the GC"_ Well, `GC.Collect` forces garbage collection – Tim Schmelter Jul 30 '18 at 14:08
  • @TimSchmelter that's what I expected too. However, I'm open to the suggestion that some recent update has debug builds keeping certain objects alive to the end of scope to avoid the optimization I mentioned, but that it isn't smart enough to realize that if I explicitly set the variable to null, I don't need to be able to inspect the old value anymore... – dlf Jul 30 '18 at 14:11
  • @TimSchmelter `GC.Collect()` forces a collection, but it's still up to the GC *when* a particular object will be deleted, especially when finalizers are involved. – Theodoros Chatzigiannakis Jul 30 '18 at 14:15
  • The information in the linked question is pretty convincing (enough so for me to consider my own a duplicate, in fact). I'm still curious why things used to work as we (incorrectly) expected and now suddenly don't, but maybe that's just trying to define undefined behavior... – dlf Jul 30 '18 at 14:34
  • @dlf Pretty much, yes. Even though they are different, both the old and the new behavior you're describing are within its specification. And conforming to its stated specification is really the only sound assumption we can make. – Theodoros Chatzigiannakis Jul 30 '18 at 14:42