5

Consider the following code:

using System;
using System.Collections.Generic;
using System.Runtime.CompilerServices;

#nullable enable

namespace ConsoleApp1
{
    class Program
    {
        static void Main()
        {
            var list    = makeList();
            var weakRef = new WeakReference(list[0]);

            list[0] = null;
            GC.Collect();
            GC.WaitForPendingFinalizers();

            Console.WriteLine(weakRef.IsAlive);
        }

        [MethodImpl(MethodImplOptions.NoInlining)]
        static List<int[]?> makeList()
        {
            return new List<int[]?> { new int[2] };
        }
    }
}
  • With either a release or a debug build on .Net Framework 4.8, that code prints False.
  • With either a release or a debug build on .Net Core 3.1, that code prints True.

What is causing this difference in behaviour? (It's causing some of our unit tests to fail.)

Note: I put the list initialisation into makeList() and turned off inlining in an attempt to make the .Net Core version work the same as the .Net Framework version, but to no avail.


[EDIT] As Hans pointed out, adding a loop fixes it.

The following code will print False:

var list    = makeList();
var weakRef = new WeakReference(list[0]);

list[0] = null;

for (int i = 0; i < 1; ++i)
    GC.Collect();

Console.WriteLine(weakRef.IsAlive);

But this will print True:

var list    = makeList();
var weakRef = new WeakReference(list[0]);

list[0] = null;

GC.Collect();
GC.Collect();
GC.Collect();
GC.Collect();

// Doesn't seem to matter how many GC.Collect() calls you do.

Console.WriteLine(weakRef.IsAlive);

This has got to be some kind of weird Jitter thing...

Matthew Watson
  • 104,400
  • 10
  • 158
  • 276
  • 1
    [Is this discussion relevant](https://github.com/dotnet/runtime/issues/8561)? – Joe Sewell Aug 19 '20 at 16:30
  • 1
    `while (weakRef.IsAlive) { GC.Collect(); GC.WaitForPendingFinalizers(); }`. I don't really want to guess why it goes through that loop only once :) – Hans Passant Aug 19 '20 at 16:44
  • @JoeSewell Yes, that seems related (they were even doing the same kind of unit test), except they also tried using a non-inlined method which fixed it for them (and it doesn't for me). – Matthew Watson Aug 19 '20 at 17:21
  • @HansPassant Isn't that the weirdest thing. I tried adding 10 separate calls to `GC.Collect()` and that doesn't fix it - but one single loop and it does. Hmmm. – Matthew Watson Aug 19 '20 at 17:22

2 Answers2

5

Just because something is allowed to be collected doesn't mean it's obligated to be collected as soon as is possible. While the language states that the GC is allowed to determine that a local variable is never read again, and thus not consider it a root, that doesn't mean you can rely on a local variable's contents being collected immediately after you last read from it.

This isn't some change between defined behavior in the runtime, this is undefined behavior in both runtimes, so differences between them is entirely acceptable.

Servy
  • 202,030
  • 26
  • 332
  • 449
  • I agree, and thus our unit test that was failing was relying on undefined behaviour. We will have to find another way to check that things are disposed correctly. – Matthew Watson Aug 19 '20 at 20:48
  • IMHO, don't test that the runtime GC's the list, test that you have set the variable to null. You know, the bit you actually have control over. – Jeremy Lakeman Sep 03 '20 at 01:52
  • @JeremyLakeman Setting it to null is a no-op. The variable is never read after you set it to null, so it accomplishes nothing. The compiler is even within its rights to remove it form the compiled code (I have no idea if it actually does, just that it would be legal for it to do so). – Servy Sep 03 '20 at 16:41
  • That's only true for a local variable about to go out of scope. There's not much point writing a unit test about locals, but you might write one to demonstrate your dispose code works. Which is where you might have been using a weak reference to prove the GC worked too. – Jeremy Lakeman Sep 04 '20 at 00:54
  • @JeremyLakeman You don't unit test the GC. That doesn't make any sense. It's Microsoft's job to test the GC, not yours. Any disposal code you write is specifically for things *not* managed by the GC, so you don't need the GC to do anything to test them. – Servy Sep 04 '20 at 01:44
  • 1
    And yet, a minor change in GC behaviour prompted the OP's question, because testing the GC is exactly what he was doing. And I agree, he shouldn't. His tests can assert that resources are disposed, any more is unnecessary. – Jeremy Lakeman Sep 04 '20 at 01:49
  • @JeremyLakeman Which is why you shouldn't write code that relies on undefined implementation details of the GC. Writing a unit test doesn't make relying on such behavior any more appropriate. That you have *some* possibility of finding out that your unreliable code is breaking doesn't make writing unreliable code any more appropriate. – Servy Sep 04 '20 at 01:54
0

I got the reference to be freed, when I removed the list variable:

using NUnit.Framework;
using System;
using System.Collections.Generic;

namespace NUnitTestProject1
{
    public class Tests
    {
        [TestCase(2, GCCollectionMode.Forced, true)]
        public void TestWeakReferenceWithList(int generation, GCCollectionMode forced, bool blocking)
        {
            static WeakReference CreateWeakReference()
            {
                return new WeakReference(new List<int[]> { new int[2] });
            }

            var x = CreateWeakReference();

            Assert.IsTrue(x.IsAlive);

            GC.Collect(generation, forced, blocking);

            Assert.IsFalse(x.IsAlive);
        }
   }
}

The following test case fails:

using NUnit.Framework;
using System;
using System.Collections.Generic;

namespace NUnitTestProject1
{
    public class Tests
    {
        [TestCase(2, GCCollectionMode.Forced, true)]
        public void TestWeakReferenceWithList(int generation, GCCollectionMode forced, bool blocking)
        {
            static List<int[]> CreateList()
            {
                return new List<int[]> { new int[2] };
            }

            WeakReference x;

            {
                var list = CreateList();

                x = new WeakReference(list);

                list = null;
            }
            
            Assert.IsTrue(x.IsAlive);

            GC.Collect(generation, forced, blocking);

            Assert.IsFalse(x.IsAlive);
        }
   }
}

If we look at the IL we can se that null is assigned to local variable 1:

IL_0003:  call       class [System.Collections]System.Collections.Generic.List`1<int32[]> NUnitTestProject1.Tests::'<TestWeakReferenceWithList>g__CreateList|0_0'()
IL_0008:  stloc.1
IL_0009:  ldloc.1
IL_000a:  newobj     instance void [System.Runtime]System.WeakReference::.ctor(object)
IL_000f:  stloc.0
IL_0010:  ldnull
IL_0011:  stloc.1
IL_0012:  nop