2

Basically I want to make sure a field in a class (in this case the _changedPoller) is Disposed as early as possible when not needed anymore. I call StopChangedPolling in the class' Dispose method and when certain events occur. What is the best, thread safe way to dispose a field only once? Is the following code ok?

private void StopChangedPolling()
{
    IDisposable changedPoller = Interlocked.CompareExchange(ref _changedPoller, null, _changedPoller);
    if (changedPoller != null)
        changedPoller.Dispose();
}

The second time this is called, _changedPoller is null and no exception is thrown, even though the documentation states that an ArgumentNullException is thrown if the address of location1 is a null pointer.

atlemann
  • 173
  • 1
  • 7

1 Answers1

2

You are not passing a null pointer, you are passing a reference to a null reference. I don't think you can provoke the exception

The address of location1 is a null pointer.

in C# because you can't create a null managed pointer in C#.

This is thread-safe but will fail to dispose in case of a thread abort. If that is not a concern for you then this is fine.

You could consider to simplify this code by using a lock but I actually think this is simpler than a lock.

You also can use Interlocked.Exchange.

Actually, there is one concern: You are reading _changedPoller in an unsynchronized way. I'm not sure what guarantees ECMA and the .NET CLR make around this. I would have to read the spec. volatile would fix that. Interlocked.Exchange would fix it too if there are no other such read usages.

usr
  • 168,620
  • 35
  • 240
  • 369
  • `You are reading _changedPoller in an unsynchronized way` Aren't the Interlocked method supposed to use a memory barrier? – Kevin Gosse Jun 18 '15 at 14:43
  • 2
    It does, but one of the arguments is simply `_changedPoller`. That's a normal read. – usr Jun 18 '15 at 14:43
  • Oh, nice catch. Even with volatile, there's a racing condition here. `Interlocked.Exchange` should definitely be used instead – Kevin Gosse Jun 18 '15 at 14:46
  • This stuff is so hard and fragile. I overlooked this at first. My recommendation to use a lock seems even wiser now. – usr Jun 18 '15 at 14:47