Thread preventing garbage collection of owner

Question

In a library I've created, I have a class, DataPort, that implements functionality similar to the .NET SerialPort class. It talks to some hardware and will raise an event whenever data comes in over that hardware. To implement this behavior, DataPort spins up a thread that is expected to have the same lifetime as the DataPort object. The problem is that when the DataPort goes out of scope, it never gets garbage collected

Now, because DataPort talks to hardware (using pInvoke) and owns some unmanaged resources, it implements IDisposable. When you call Dispose on the object, everything happens correctly. The DataPort gets rid of all of its unmanaged resources and kills the worker thread and goes away. If you just let DataPort go out of scope, however, the garbage collector will never call the finalizer and the DataPort will stay alive in memory forever. I know this is happening for two reasons:

1. A breakpoint in the finalizer never gets hit
2. SOS.dll tells me that the DataPort is still alive

Sidebar: Before we go any further, I'll say that yes, I know the answer is "Call Dispose() Dummy!" but I think that even if you let all references go out of scope, the right thing should happen eventually and the garbage collector should get rid of the DataPort

Back to the Issue: Using SOS.dll, I can see that the reason my DataPort isn't being garbage collected is because the thread that it spun up still has a reference to the DataPort object - through the implicit "this" parameter of the instance method that the thread is running. The running worker thread will not be garbage collected, so any references that are in the scope of the running worker thread are also not eligible for garbage collection.

The thread itself runs basically the following code:

public void WorkerThreadMethod(object unused)
{
  ManualResetEvent dataReady = pInvoke_SubcribeToEvent(this.nativeHardwareHandle);
  for(;;)
  {
    //Wait here until we have data, or we got a signal to terminate the thread because we're being disposed
    int signalIndex = WaitHandle.WaitAny(new WaitHandle[] {this.dataReady, this.closeSignal});
    if(signalIndex == 1) //closeSignal is at index 1
    {
      //We got the close signal.  We're being disposed!
      return; //This will stop the thread
    }
    else
    {
      //Must've been the dataReady signal from the hardware and not the close signal.
      this.ProcessDataFromHardware();
      dataReady.Reset()
    }
  }
}

The Dispose method contains the following (relevant) code:

public void Dispose()
{
  closeSignal.Set();
  workerThread.Join();
}

Because the thread is a gc root and it holds a reference to the DataPort, the DataPort is never eligible for the garbage collection. Because the finalizer is never called, we never send the close signal to the worker thread. Because the worker thread never gets the close signal, it keeps going forever and holding that reference. ACK!

The only answer I can think of to this problem is to get rid of the 'this' parameter on the WorkerThread method (detailed below in the answers). Can anybody else think of another option? There must be a better way to create an object with a thread that has the same lifetime of the object! Alternatively, can this be done without a separate thread? I chose this particular design based on this post over at the msdn forums that describe some of the internal implementation details of the regular .NET serial port class

Update a bit of extra information from the comments:

• The thread in question has IsBackground set to true
• The unmanaged resources mentioned above don't affect the problem. Even if everything in the example used managed resources, I would still see the same issue

Answer 1

To get rid of the implicit "This" parameter, I changed the worker thread method around a bit and passed in the "this" reference as a parameter:

public static void WorkerThreadMethod(object thisParameter)
{
  //Extract the things we need from the parameter passed in (the DataPort)
  //dataReady used to be 'this.dataReady' and closeSignal used to be
  //'this.closeSignal'
  ManualResetEvent dataReady = ((DataPort)thisParameter).dataReady;
  WaitHandle closeSignal = ((DataPort)thisParameter).closeSignal;

  thisParameter = null; //Forget the reference to the DataPort

  for(;;)
  {
    //Same as before, but without "this" . . .
  }
}

Shockingly, this did not solve the problem!

Going back to SOS.dll, I saw that there was still a reference to my DataPort being held by a ThreadHelper object. Apparently when you spin up a worker thread by doing Thread.Start(this);, it creates a private ThreadHelper object with the same lifetime as the thread that holds onto the reference that you passed in to the Start method (I'm inferring). That leaves us with the same problem. Something is holding a reference to DataPort. Let's give this one more try:

//Code that starts the thread:
  Thread.Start(new WeakReference(this))
//. . .
public static void WorkerThreadMethod(object weakThisReference)
{
  DataPort strongThisReference= (DataPort)((WeakReference)weakThisReference).Target;

  //Extract the things we need from the parameter passed in (the DataPort)
  ManualResetEvent dataReady = strongThisReferencedataReady;
  WaitHandle closeSignal = strongThisReference.closeSignal;

  strongThisReference= null; //Forget the reference to the DataPort.

  for(;;)
  {
    //Same as before, but without "this" . . .
  }
}

Now we're OK. The ThreadHelper that gets created holds onto a WeakReference, which won't affect garbage collection. We extract only the data we need from the DataPort at the beginning of the worker thread and then intentionally lose all references to the DataPort. This is OK in this application because the parts of it that we grab don't change over the lifetime of the DataPort. Now, when the top level application loses all references to the DataPort, it's eligible for garbage collection. The GC will run the finalizer which will call the Dispose method which will kill the worker thread. Everything is happy.

However, this is a real pain to do (or at least get right)! Is there a better way to make an object that owns a thread with the same lifetime as that object? Alternatively, is there a way to do this without the thread?

Epilogue: It would be great if instead of having a thread that spends most of its time doing WaitHandle.WaitAny(), you could have some sort of wait handle that didn't need it's own thread, but would fire a continuation on a Threadpool thread once it's triggered. Like, if the hardware DLL could just call a delegate every time there's new data instead of signaling an event, but I don't control that dll.

Answer 2

I believe that the problem is not in the code you have shown but in the code using this serial port wrapper class. If you don't have a "using" statement there, see http://msdn.microsoft.com/en-us/library/yh598w02.aspx, you don't have deterministic cleanup behaviour. Instead, you then rely on the garbage collector but that will never reap an object that is still referenced, and all stack-variables of a thread (whether as normal parameter or this-pointer) count as references.