Cette implémentation IDisposable est-elle correcte?

Question

Je ne me souviens jamais de toutes les règles d'implémentation de l'interface IDisposable, j'ai donc essayé de créer une classe de base qui s'occupe de tout cela et qui rend l'IDisposable facile à implémenter. Je voulais juste entendre votre opinion si cette implémentation est correcte telle quelle ou si vous voyez quelque chose que je pourrais améliorer. L'utilisateur de cette classe de base est supposé en dériver, puis implémenter les deux méthodes abstraites ReleaseManagedResources() et ReleaseUnmanagedResources(). Donc, voici le code:

public abstract class Disposable : IDisposable 
{ 
    private bool _isDisposed; 
    private readonly object _disposeLock = new object(); 

    /// <summary> 
    /// called by users of this class to free managed and unmanaged resources 
    /// </summary> 
    public void Dispose() { 
     DisposeManagedAndUnmanagedResources(); 
    } 

    /// <summary> 
    /// finalizer is called by garbage collector to free unmanaged resources 
    /// </summary> 
    ~Disposable() { //finalizer of derived class will automatically call it's base finalizer 
     DisposeUnmanagedResources(); 
    } 

    private void DisposeManagedAndUnmanagedResources() { 
     lock (_disposeLock) //make thread-safe 
      if (!_isDisposed) { //make sure only called once 
       try { //suppress exceptions 
        ReleaseManagedResources(); 
        ReleaseUnmanagedResources(); 
       } 
       finally { 
        GC.SuppressFinalize(this); //remove from finalization queue since cleaup already done, so it's not necessary the garbage collector to call Finalize() anymore 
        _isDisposed = true; 
       } 
      } 
    } 

    private void DisposeUnmanagedResources() { 
     lock (_disposeLock) //make thread-safe since at least the finalizer runs in a different thread 
      if (!_isDisposed) { //make sure only called once 
       try { //suppress exceptions 
        ReleaseUnmanagedResources(); 
       } 
       finally { 
        _isDisposed = true; 
       } 
      } 
    } 

    protected abstract void ReleaseManagedResources(); 
    protected abstract void ReleaseUnmanagedResources(); 

} 

Answer 1

Vous accédez à l'objet géré _disposeLock dans le finaliseur. Il a peut-être déjà été collecté par les ordures d'ici là. Vous ne savez pas quelles seraient les implications de cela car vous ne l'utilisez que pour verrouiller.

La sécurité du fil semble être excessive. Je ne pense pas que vous ayez à vous soucier des conflits entre le thread GC et votre thread d'application.

Sinon, cela semble correct.

Answer 2

EDIT Oui, j'ai mal lu la partie de séparer et les ressources gérées disposant non gérés (toujours sur la première tasse de café).

Cependant, le verrouillage n'est presque certainement pas nécessaire. Oui, lors d'une mise au rebut passive, le code sera exécuté sur le thread finalizer qui est différent du thread sur lequel il était initialement exécuté. Cependant, si l'objet est en train d'être finalisé de cette manière, le CLR a déjà déterminé qu'il n'existe aucune référence à cet objet et donc l'a collecté. Donc, il n'y a pas d'autre endroit qui peut appeler votre disposer à ce moment-là et donc il n'y a pas de raison pour un verrou.

Un couple d'autres commentaires de style. Pourquoi les méthodes sont-elles abstraites?

Pourquoi? Ce faisant, vous forcez les classes dérivées à implémenter des méthodes pour disposer des ressources gérées et non gérées, même si elles n'en disposent pas. Il est vrai qu'il n'y a aucun intérêt à dériver de cette classe si vous n'avez rien à éliminer. Mais je pense que c'est assez commun d'avoir seulement l'un ou l'autre mais pas les deux. Je les rendrais virtuels ou abstraits.

Vous évitez également d'éliminer deux fois mais vous ne faites rien pour avertir le développeur qu'il dispose d'un double objet. Je me rends compte que la documentation MSDN dit que la double disposition devrait être essentiellement une interdiction, mais en même temps dans quelles circonstances cela devrait-il être légal? En général, c'est une mauvaise idée d'accéder à un objet après qu'il a été éliminé. Une double disposition nécessite la réutilisation d'un objet disposé et est probablement un bug (ceci peut arriver si la finalisation n'est pas supprimée dans une disposition active mais c'est aussi une mauvaise idée). Si je mettais en œuvre ceci je jetterais sur double disposition pour avertir le développeur qu'ils utilisaient un objet incorrectement (c'est-à-dire l'utiliser après qu'il était déjà disposé).

Answer 3

Je ne peux pas vraiment commenter l'exactitude de votre code, mais je me demande si vous trouverez que la classe jetable à la base de votre arbre d'héritage est aussi flexible qu'elle doit l'être. Ce ne sera pas très utile lorsque vous voulez hériter de quelque chose que vous ne possédez pas. Je pense qu'IDispoasble est laissé en tant qu'interface en raison des nombreux domaines différents dans lesquels il pourrait être utilisé. Fournir une implémentation concrète au pied de la hiérarchie de l'héritage serait ... limitatif.

Answer 4

Si vous êtes préoccupé par la sécurité de fil puis j'utiliser les opérations emboîtés légers plutôt qu'un verrou lourd:

class MyClass: IDispose { 
    int disposed = 0; 

    public void Dispose() 
    { 
    Dispose(true); 
    GC.SuppressFinalize(this); 
    } 

    public virtual void Dispose(bool disposing) 
    { 
    if (0 == Thread.InterlockedCompareExchange(ref disposed, 1, 0)) 
    { 
     if (disposing) 
     { 
      // release managed resources here 
     } 
     // release unmanaged resources here 
    } 
    } 

    ~MyClass() 
    { 
    Dispose(false); 
    } 
}