2

Here is the scenario:

I have an object called a Transaction that needs to make sure that only one entity has permission to edit it at any given time.

In order to facilitate a long-lived lock, I have the class generating a token object that can be used to make the edits.

You would use it like this:

var transaction = new Transaction();

using (var tlock = transaction.Lock())
{
    transaction.Update(data, tlock);
}

Now, I want the TransactionLock class to implement IDisposable so that its usage can be clear. But, I don't have any unmanaged resources to dispose. however, the TransctionLock object itself is a sort of "unmanaged resource" in the sense that the CLR doesn't know how to properly finalize it.

All of this would be fine and dandy, I would just use IDisposable and be done with it.

However, my issue comes when I try to do this in the finalizer:

~TransactionLock()
{
    this.Dispose(false);
}

I want the finalizer to release the transaction from the lock, if possible. How, in the finalizer, do I detect if the parent transaction (this.transaction) has already been finalized?

Is there a better pattern I should be using?

Also, the Transaction class itself needn't be disposable, because it doesn't maintain a reference to the lock, and doesn't care whether or not it is unlocked when it goes to the grave.


The Transaction class looks something like this:

public sealed class Transaction
{
    private readonly object lockMutex = new object();

    private TransactionLock currentLock;

    public TransactionLock Lock()
    {
        lock (this.lockMutex)
        {
            if (this.currentLock != null)
                throw new InvalidOperationException(/* ... */);

            this.currentLock = new TransactionLock(this);
            return this.currentLock;
        }
    }

    public void Update(object data, TransactionLock tlock)
    {
        lock (this.lockMutex)
        {
            this.ValidateLock(tlock);

            // ...
        }
    }

    internal void ValidateLock(TransactionLock tlock)
    {
        if (this.currentLock == null)
            throw new InvalidOperationException(/* ... */);

        if (this.currentLock != tlock)
            throw new InvalidOperationException(/* ... */);
    }

    internal void Unlock(TransactionLock tlock)
    {
        lock (this.lockMutex)
        {
            this.ValidateLock(tlock);

            this.currentLock = null;
        }
    }
}

And the Dispose(bool) code for the TransactionLock:

private void Dispose(bool disposing)
{
    if (disposing)
    {
        if (this.Transaction != null)
        {
            this.Transaction.Unlock(this);
            this.Transaction = null;
        }
    }
}
John Gietzen
  • 48,783
  • 32
  • 145
  • 190
  • Under what circumstances could a `TransactionLock` realistically be abandoned without `Dispose` having been called upon it? I can't see any case in which the finalizer would ever be able to do anything useful. – supercat Jun 28 '12 at 18:30

2 Answers2

2

This was discussed before. Your case is much easier though, you are also implementing the finalizer. That's fundamentally wrong, you are hiding a bug in the client code. Beware that finalizers run on a separate thread. Debugging a consistent deadlock is much easier than dealing with locks that disappear randomly and asynchronously.

Recommendation: follow the .NET framework lead: don't help too much. Microsoft abandoned the Synchronized method for the same reason.

Community
  • 1
  • 1
Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536
  • This is not *quite* the same as was mentioned before. In my case, the token object being disposed is actually used as a validation mechanism, and needs to be cleaned properly. The stages of what needs to go on in my scenario are exactly the same as a native handle. – John Gietzen Apr 05 '10 at 22:50
  • Ok, well, is there any way to get the `using`-like semantics without actually implementing `IDisposable`? I'm pretty sure the answer there is no. In that case, is there any way to deterministically fire a Finalizer, instead? – John Gietzen Apr 05 '10 at 23:04
  • Also, I just posted my Dispose code, which, as you may see, does not free the lock (since I am unable to prove whether the Transaction has been finalized). That means that this will lead to the consistent deadlock (or, actually, `InvalidOperationException`) that you mentioned. What that actually means is that my finalizer is always a waste, since it never does any actual disposing. I would *like* it to dispose of the lock, but I guess, as you have suggested, that that is "Helping too much"... – John Gietzen Apr 05 '10 at 23:10
1

How, in the finalizer, do I detect if the parent transaction (this.transaction) has already been finalized?

This is possible by keeping a _disposed boolean field in Transaction and exposing it through an IsDisposed read-only property. This is standard practice.

   ~TransactionLock()
    {
        this.Dispose(false);
    }

Is there a better pattern I should be using?

If it is correct that TransactionLock has no unmanaged resources then just omit the destructor (finalizer). It has no function but it does have a considerable cost.

Edit: If I read correctly, Unlock does not cut the link from TransactionLock to TTransaction, meaning that an old locks Dispose(bool) will be called though the destructor. It's not clear if that is safe.

The question would be a bit more complete with the code of TransactionLock.Dispose(bool)


Also, the Transaction class itself needn't be disposable, because it doesn't maintain a reference to the lock, and doesn't care whether or not it is unlocked when it goes to the grave.

From this it follows that when a TransactionLock is collected, it could only be holding a ref to a Transaction that is also being collected. No need to interfere with destructors here, that would not solve anything and could only create problems you don't need.

H H
  • 263,252
  • 30
  • 330
  • 514
  • I guess, then what I'm really trying to do is emulate RAII in C#, by forcing (or, suggesting) a Dispose() rather than the Destructor / Finalizer. Is there a way to implement a finalizer that is called immediately when an object goes out of scope? – John Gietzen Apr 05 '10 at 22:53
  • @John: for RAII, just implement Dispose() and use `using`. Still no need for a destructor. – H H Apr 06 '10 at 10:14