Dovresti implementare IDisposable.Dispose() in modo che non getti mai?

Question

Per il meccanismo equivalente in C++ (il distruttore), il consiglio è che it should usually not throw any exceptions. Questo è principalmente dovuto al fatto che così facendo si potrebbe interrompere il processo, che è solo raramente una buona strategia.

Nello scenario equivalente NET ...

1. Una prima eccezione viene generata
2. Un blocco finally viene eseguita a seguito della prima eccezione
3. Il blocco finally chiama Dispose (metodo)
metodo
4. Il Dispose() genera una seconda eccezione

... il processo non termina immediatel y. Tuttavia, si perdono le informazioni perché .NET sostituisce in modo non trascurabile la prima eccezione con la seconda. Un blocco catch da qualche parte nello stack di chiamate non vedrà quindi mai la prima eccezione. Tuttavia, di solito uno è più interessato alla prima eccezione, poiché normalmente dà migliori indizi sul motivo per cui le cose hanno iniziato a non funzionare.

Dato .NET manca di un meccanismo per rilevare se il codice viene eseguito, mentre un'eccezione è in attesa, sembra che ci sono solo due scelte come IDisposable realizzabilità:

• ingoiare sempre tutte le eccezioni che si verificano all'interno Dispose(). Non va bene dato che potresti anche finire per inghiottire OutOfMemoryException, ExecutionEngineException, ecc. Che di solito preferisco lasciare abbattere il processo quando si verificano senza un'altra eccezione già in sospeso.
• Lascia che tutte le eccezioni si propagino fuori da Dispose(). Non va bene dato che potresti perdere informazioni sulla causa principale di un problema, vedi sopra.

Quindi, qual è il minore dei due mali? C'è un modo migliore?

EDIT: Per chiarire, non sto parlando di gettare attivamente eccezioni da Dispose() o no, sto parlando di lasciare eccezioni generate con metodi chiamati da Dispose() si diffondono fuori Dispose() o no , per esempio:

using System; 
using System.Net.Sockets; 

public sealed class NntpClient : IDisposable 
{ 
    private TcpClient tcpClient; 

    public NntpClient(string hostname, int port) 
    { 
     this.tcpClient = new TcpClient(hostname, port); 
    } 

    public void Dispose() 
    { 
     // Should we implement like this or leave away the try-catch? 
     try 
     { 
      this.tcpClient.Close(); // Let's assume that this might throw 
     } 
     catch 
     { 
     } 
    } 
} 

Answer 1

direi che deglutizione è il minore dei due mali in questo scenario, come è meglio per aumentare il originaleException - avvertimento: a meno, forse la mancata modo pulito smaltire è di per sé dannatamente critico (forse se un TransactionScope non può essere smaltito, poiché ciò potrebbe indicare un errore di rollback).

Vedere here per ulteriori riflessioni su questo - tra cui un metodo involucro/estensione idea:

using(var foo = GetDodgyDisposableObject().Wrap()) { 
    foo.BaseObject.SomeMethod(); 
    foo.BaseObject.SomeOtherMethod(); // etc 
} // now exits properly even if Dispose() throws 

Naturalmente, si potrebbe anche fare qualche stranezza in cui si ri-un'eccezione composito con sia l'originale e la seconda (Dispose()) eccezione - ma pensa: potresti avere più blocchi using ... diventerebbe rapidamente ingestibile. In realtà, l'eccezione originale è quella interessante.

Answer 2

Dispose deve essere progettato per fare lo scopo, smaltendo l'oggetto.Questa attività è sicura e non genera eccezioni la maggior parte del tempo. Se ti vedi a lanciare eccezioni da Dispose, dovresti pensarci due volte per vedere se stai facendo troppe cose in esso. Oltre a questo, penso che lo Dispose debba essere trattato come tutti gli altri metodi: gestisci se puoi fare qualcosa con esso, lascia che bolla se non puoi.

EDIT: Per l'esempio specificato, vorrei scrivere il codice in modo che il mio codice non causa un'eccezione, ma di compensazione il TcpClient up potrebbe causare un'eccezione, che dovrebbe essere valido per propagare a mio parere (o per gestire e rigenerare come eccezione più generico, come qualsiasi metodo):

public void Dispose() { 
    if (tcpClient != null) 
    tcpClient.Close(); 
} 

Tuttavia, proprio come qualsiasi altro metodo, se si sa tcpClient.Close() potrebbe generare un'eccezione che deve essere ignorato (non importa) o dovrebbe essere rappresentato da un altro oggetto eccezione, si potrebbe desiderare di prenderlo.

Answer 3

Probabilmente utilizzerei la registrazione per acquisire dettagli sulla prima eccezione, quindi consentire di sollevare la seconda eccezione.

Answer 4

Il Framework Design Guidelines (2 ^° ed) presenta questo come (§9.4.1):

EVITARE un'eccezione dall'interno Dispose (bool) tranne in critiche situazioni in cui il processo contenente è stato danneggiato (perdite, stato incoerente condiviso, ecc.).

Commentary [Edit]:

• Ci sono linee guida, non regole dure. E questa è una "EVITARE" non una "NON" linea guida. Come notato (nei commenti) il Framework rompe questa (e altre) linee guida in alcuni punti. Il trucco è sapere quando rompere una linea guida. Questo, in molti modi, è la differenza tra un Journeyman e un Master.
• Se parte del clean-up può fallire, è necessario fornire un metodo Close che genererà eccezioni in modo che il chiamante possa gestirle.
• Se si segue il modello di eliminazione (e si dovrebbe essere se il tipo contiene direttamente alcune risorse non gestite), è possibile chiamare Dispose(bool) dal finalizzatore, lanciare da un finalizzatore è una cattiva idea e blocca la finalizzazione di altri oggetti .

mio vista: eccezioni fuga da smaltire siano esclusivamente quelli, come nella guida, che sufficientemente catastrofici che nessun ulteriore funzione affidabile è possibile dal processo corrente.

Answer 5

Il rilascio di risorse dovrebbe essere un'operazione "sicura": in fondo, come posso recuperare dal non essere in grado di rilasciare una risorsa? quindi non è logico lanciare un'eccezione da Dispose.

Tuttavia, se scopro all'interno di Dispose che lo stato del programma è danneggiato, è meglio lanciare l'eccezione, quindi inghiottirlo, è meglio schiacciare ora per continuare a correre e produrre risultati errati.

Answer 6

Peccato che Microsoft non abbia fornito un parametro Exception a Dispose, con l'intenzione di essere incapsulato come InnerException nel caso in cui l'eliminazione stessa generi un'eccezione.Per essere sicuro, l'uso efficace di tale parametro richiederebbe l'uso di un blocco del filtro delle eccezioni, che C# non supporta, ma forse l'esistenza di un tale parametro avrebbe potuto motivare i progettisti C# a fornire tale caratteristica? Una bella variazione che mi piacerebbe vedere sarebbe l'aggiunta di un "parametro" di eccezione a un blocco Finally, ad es.

 
    finally Exception ex: // In C# 
    Finally Ex as Exception ' In VB 

che comportarsi come un normale blocco Finally se non che 'ex' sarebbe nullo/Nulla se la 'Prova' corse a compimento, o da ostacolare l'eccezione generata se non lo ha fatto. Peccato che non ci sia modo di far sì che il codice esistente utilizzi tale caratteristica.

Answer 7

Ecco un modo per rimuovere in modo discreto eventuali eccezioni generate dal contenuto dello using o dallo Dispose.

Codice originale:

using (var foo = new DisposableFoo()) 
{ 
    codeInUsing(); 
} 

Poi qui è il codice che getterà se uno codeInUsing() tiri o foo.Dispose() tiri o entrambi tiro, e permetterà di vedere la prima eccezione (a volte avvolto come un InnerExeption, a seconda):

var foo = new DisposableFoo(); 
Helpers.DoActionThenDisposePreservingActionException(
    () => 
    { 
     codeInUsing(); 
    }, 
    foo); 

Non è bello ma non troppo male.

Ecco il codice per implementare questo. L'ho impostato in modo che lo solo funzioni come descritto quando il debugger non è collegato, perché quando viene collegato il debugger sono più preoccupato che si interrompa nel posto giusto alla prima eccezione. È possibile modificare secondo necessità.

public static void DoActionThenDisposePreservingActionException(Action action, IDisposable disposable) 
{ 
    bool exceptionThrown = true; 
    Exception exceptionWhenNoDebuggerAttached = null; 
    bool debuggerIsAttached = Debugger.IsAttached; 
    ConditionalCatch(
     () => 
     { 
      action(); 
      exceptionThrown = false; 
     }, 
     (e) => 
     { 
      exceptionWhenNoDebuggerAttached = e; 
      throw new Exception("Catching exception from action(), see InnerException", exceptionWhenNoDebuggerAttached); 
     }, 
     () => 
     { 
      Exception disposeExceptionWhenExceptionAlreadyThrown = null; 
      ConditionalCatch(
       () => 
       { 
        disposable.Dispose(); 
       }, 
       (e) => 
       { 
        disposeExceptionWhenExceptionAlreadyThrown = e; 
        throw new Exception("Caught exception in Dispose() while unwinding for exception from action(), see InnerException for action() exception", 
         exceptionWhenNoDebuggerAttached); 
       }, 
       null, 
       exceptionThrown && !debuggerIsAttached); 
     }, 
     !debuggerIsAttached); 
} 

public static void ConditionalCatch(Action tryAction, Action<Exception> conditionalCatchAction, Action finallyAction, bool doCatch) 
{ 
    if (!doCatch) 
    { 
     try 
     { 
      tryAction(); 
     } 
     finally 
     { 
      if (finallyAction != null) 
      { 
       finallyAction(); 
      } 
     } 
    } 
    else 
    { 
     try 
     { 
      tryAction(); 
     } 
     catch (Exception e) 
     { 
      if (conditionalCatchAction != null) 
      { 
       conditionalCatchAction(e); 
      } 
     } 
     finally 
     { 
      if (finallyAction != null) 
      { 
       finallyAction(); 
      } 
     } 
    } 
} 

Answer 8

ci sono diverse strategie per la moltiplicazione o deglutire eccezioni al metodo di Dispose, possibilmente in base al fatto un'eccezione unhanded è stato anche gettato dalla logica principale. La soluzione migliore sarebbe lasciare la decisione al chiamante, in base alle proprie esigenze specifiche.Ho implementato un metodo di estensione generica che fa questo, che offre:

• il default using semantica di moltiplicazione Dispose eccezioni
• Marc Gravell's suggestion di deglutizione sempre Dispose eccezioni
• maxyfc's alternative di soli deglutizione Dispose eccezioni quando c'è un'eccezione dalla logica principale che altrimenti andrebbe persa
• Daniel Chambers's approach di avvolgere più eccezioni in un AggregateException
• un approccio simile che avvolge sempre tutte le eccezioni in un AggregateException (come Task.Wait fa)

Questo è il mio metodo di estensione:

/// <summary> 
/// Provides extension methods for the <see cref="IDisposable"/> interface. 
/// </summary> 
public static class DisposableExtensions 
{ 
    /// <summary> 
    /// Executes the specified action delegate using the disposable resource, 
    /// then disposes of the said resource by calling its <see cref="IDisposable.Dispose()"/> method. 
    /// </summary> 
    /// <typeparam name="TDisposable">The type of the disposable resource to use.</typeparam> 
    /// <param name="disposable">The disposable resource to use.</param> 
    /// <param name="action">The action to execute using the disposable resource.</param> 
    /// <param name="strategy"> 
    /// The strategy for propagating or swallowing exceptions thrown by the <see cref="IDisposable.Dispose"/> method. 
    /// </param> 
    /// <exception cref="ArgumentNullException"><paramref name="disposable"/> or <paramref name="action"/> is <see langword="null"/>.</exception> 
    public static void Using<TDisposable>(this TDisposable disposable, Action<TDisposable> action, DisposeExceptionStrategy strategy) 
     where TDisposable : IDisposable 
    { 
     ArgumentValidate.NotNull(disposable, nameof(disposable)); 
     ArgumentValidate.NotNull(action, nameof(action)); 
     ArgumentValidate.IsEnumDefined(strategy, nameof(strategy)); 

     Exception mainException = null; 

     try 
     { 
      action(disposable); 
     } 
     catch (Exception exception) 
     { 
      mainException = exception; 
      throw; 
     } 
     finally 
     { 
      try 
      { 
       disposable.Dispose(); 
      } 
      catch (Exception disposeException) 
      { 
       switch (strategy) 
       { 
        case DisposeExceptionStrategy.Propagate: 
         throw; 

        case DisposeExceptionStrategy.Swallow: 
         break; // swallow exception 

        case DisposeExceptionStrategy.Subjugate: 
         if (mainException == null) 
          throw; 
         break; // otherwise swallow exception 

        case DisposeExceptionStrategy.AggregateMultiple: 
         if (mainException != null) 
          throw new AggregateException(mainException, disposeException); 
         throw; 

        case DisposeExceptionStrategy.AggregateAlways: 
         if (mainException != null) 
          throw new AggregateException(mainException, disposeException); 
         throw new AggregateException(disposeException); 
       } 
      } 

      if (mainException != null && strategy == DisposeExceptionStrategy.AggregateAlways) 
       throw new AggregateException(mainException); 
     } 
    } 
} 

Queste sono le strategie messe in atto:

/// <summary> 
/// Identifies the strategy for propagating or swallowing exceptions thrown by the <see cref="IDisposable.Dispose"/> method 
/// of an <see cref="IDisposable"/> instance, in conjunction with exceptions thrown by the main logic. 
/// </summary> 
/// <remarks> 
/// This enumeration is intended to be used from the <see cref="DisposableExtensions.Using"/> extension method. 
/// </remarks> 
public enum DisposeExceptionStrategy 
{ 
    /// <summary> 
    /// Propagates any exceptions thrown by the <see cref="IDisposable.Dispose"/> method. 
    /// If another exception was already thrown by the main logic, it will be hidden and lost. 
    /// This behaviour is consistent with the standard semantics of the <see langword="using"/> keyword. 
    /// </summary> 
    /// <remarks> 
    /// <para> 
    /// According to Section 8.10 of the C# Language Specification (version 5.0): 
    /// </para> 
    /// <blockquote> 
    /// If an exception is thrown during execution of a <see langword="finally"/> block, 
    /// and is not caught within the same <see langword="finally"/> block, 
    /// the exception is propagated to the next enclosing <see langword="try"/> statement. 
    /// If another exception was in the process of being propagated, that exception is lost. 
    /// </blockquote> 
    /// </remarks> 
    Propagate, 

    /// <summary> 
    /// Always swallows any exceptions thrown by the <see cref="IDisposable.Dispose"/> method, 
    /// regardless of whether another exception was already thrown by the main logic or not. 
    /// </summary> 
    /// <remarks> 
    /// This strategy is presented by Marc Gravell in 
    /// <see href="http://blog.marcgravell.com/2008/11/dontdontuse-using.html">don't(don't(use using))</see>. 
    /// </remarks> 
    Swallow, 

    /// <summary> 
    /// Swallows any exceptions thrown by the <see cref="IDisposable.Dispose"/> method 
    /// if and only if another exception was already thrown by the main logic. 
    /// </summary> 
    /// <remarks> 
    /// This strategy is suggested in the first example of the Stack Overflow question 
    /// <see href="https://stackoverflow.com/q/1654487/1149773">Swallowing exception thrown in catch/finally block</see>. 
    /// </remarks> 
    Subjugate, 

    /// <summary> 
    /// Wraps multiple exceptions, when thrown by both the main logic and the <see cref="IDisposable.Dispose"/> method, 
    /// into an <see cref="AggregateException"/>. If just one exception occurred (in either of the two), 
    /// the original exception is propagated. 
    /// </summary> 
    /// <remarks> 
    /// This strategy is implemented by Daniel Chambers in 
    /// <see href="http://www.digitallycreated.net/Blog/51/c%23-using-blocks-can-swallow-exceptions">C# Using Blocks can Swallow Exceptions</see> 
    /// </remarks> 
    AggregateMultiple, 

    /// <summary> 
    /// Always wraps any exceptions thrown by the main logic and/or the <see cref="IDisposable.Dispose"/> method 
    /// into an <see cref="AggregateException"/>, even if just one exception occurred. 
    /// </summary> 
    /// <remarks> 
    /// This strategy is similar to behaviour of the <see cref="Task.Wait()"/> method of the <see cref="Task"/> class 
    /// and the <see cref="Task{TResult}.Result"/> property of the <see cref="Task{TResult}"/> class: 
    /// <blockquote> 
    /// Even if only one exception is thrown, it is still wrapped in an <see cref="AggregateException"/> exception. 
    /// </blockquote> 
    /// </remarks> 
    AggregateAlways, 
} 

Esempio di utilizzo:

new FileStream(Path.GetTempFileName(), FileMode.Create) 
    .Using(strategy: DisposeExceptionStrategy.Subjugate, action: fileStream => 
    { 
     // Access fileStream here 
     fileStream.WriteByte(42); 
     throw new InvalidOperationException(); 
    }); 
    // Any Dispose() exceptions will be swallowed due to the above InvalidOperationException 

Aggiornamento: Se è necessario supportare i delegati che restituiscono valori e/o sono asincrone, allora si potrebbe utilizzare questi sovraccarichi:

/// <summary> 
/// Provides extension methods for the <see cref="IDisposable"/> interface. 
/// </summary> 
public static class DisposableExtensions 
{ 
    /// <summary> 
    /// Executes the specified action delegate using the disposable resource, 
    /// then disposes of the said resource by calling its <see cref="IDisposable.Dispose()"/> method. 
    /// </summary> 
    /// <typeparam name="TDisposable">The type of the disposable resource to use.</typeparam> 
    /// <param name="disposable">The disposable resource to use.</param> 
    /// <param name="strategy"> 
    /// The strategy for propagating or swallowing exceptions thrown by the <see cref="IDisposable.Dispose"/> method. 
    /// </param> 
    /// <param name="action">The action delegate to execute using the disposable resource.</param> 
    public static void Using<TDisposable>(this TDisposable disposable, DisposeExceptionStrategy strategy, Action<TDisposable> action) 
     where TDisposable : IDisposable 
    { 
     ArgumentValidate.NotNull(disposable, nameof(disposable)); 
     ArgumentValidate.NotNull(action, nameof(action)); 
     ArgumentValidate.IsEnumDefined(strategy, nameof(strategy)); 

     disposable.Using(strategy, disposableInner => 
     { 
      action(disposableInner); 
      return true; // dummy return value 
     }); 
    } 

    /// <summary> 
    /// Executes the specified function delegate using the disposable resource, 
    /// then disposes of the said resource by calling its <see cref="IDisposable.Dispose()"/> method. 
    /// </summary> 
    /// <typeparam name="TDisposable">The type of the disposable resource to use.</typeparam> 
    /// <typeparam name="TResult">The type of the return value of the function delegate.</typeparam> 
    /// <param name="disposable">The disposable resource to use.</param> 
    /// <param name="strategy"> 
    /// The strategy for propagating or swallowing exceptions thrown by the <see cref="IDisposable.Dispose"/> method. 
    /// </param> 
    /// <param name="func">The function delegate to execute using the disposable resource.</param> 
    /// <returns>The return value of the function delegate.</returns> 
    public static TResult Using<TDisposable, TResult>(this TDisposable disposable, DisposeExceptionStrategy strategy, Func<TDisposable, TResult> func) 
     where TDisposable : IDisposable 
    { 
     ArgumentValidate.NotNull(disposable, nameof(disposable)); 
     ArgumentValidate.NotNull(func, nameof(func)); 
     ArgumentValidate.IsEnumDefined(strategy, nameof(strategy)); 

#pragma warning disable 1998 
     var dummyTask = disposable.UsingAsync(strategy, async (disposableInner) => func(disposableInner)); 
#pragma warning restore 1998 

     return dummyTask.GetAwaiter().GetResult(); 
    } 

    /// <summary> 
    /// Executes the specified asynchronous delegate using the disposable resource, 
    /// then disposes of the said resource by calling its <see cref="IDisposable.Dispose()"/> method. 
    /// </summary> 
    /// <typeparam name="TDisposable">The type of the disposable resource to use.</typeparam> 
    /// <param name="disposable">The disposable resource to use.</param> 
    /// <param name="strategy"> 
    /// The strategy for propagating or swallowing exceptions thrown by the <see cref="IDisposable.Dispose"/> method. 
    /// </param> 
    /// <param name="asyncFunc">The asynchronous delegate to execute using the disposable resource.</param> 
    /// <returns>A task that represents the asynchronous operation.</returns> 
    public static Task UsingAsync<TDisposable>(this TDisposable disposable, DisposeExceptionStrategy strategy, Func<TDisposable, Task> asyncFunc) 
     where TDisposable : IDisposable 
    { 
     ArgumentValidate.NotNull(disposable, nameof(disposable)); 
     ArgumentValidate.NotNull(asyncFunc, nameof(asyncFunc)); 
     ArgumentValidate.IsEnumDefined(strategy, nameof(strategy)); 

     return disposable.UsingAsync(strategy, async (disposableInner) => 
     { 
      await asyncFunc(disposableInner); 
      return true; // dummy return value 
     }); 
    } 

    /// <summary> 
    /// Executes the specified asynchronous function delegate using the disposable resource, 
    /// then disposes of the said resource by calling its <see cref="IDisposable.Dispose()"/> method. 
    /// </summary> 
    /// <typeparam name="TDisposable">The type of the disposable resource to use.</typeparam> 
    /// <typeparam name="TResult">The type of the return value of the asynchronous function delegate.</typeparam> 
    /// <param name="disposable">The disposable resource to use.</param> 
    /// <param name="strategy"> 
    /// The strategy for propagating or swallowing exceptions thrown by the <see cref="IDisposable.Dispose"/> method. 
    /// </param> 
    /// <param name="asyncFunc">The asynchronous function delegate to execute using the disposable resource.</param> 
    /// <returns> 
    /// A task that represents the asynchronous operation. 
    /// The task result contains the return value of the asynchronous function delegate. 
    /// </returns> 
    public static async Task<TResult> UsingAsync<TDisposable, TResult>(this TDisposable disposable, DisposeExceptionStrategy strategy, Func<TDisposable, Task<TResult>> asyncFunc) 
     where TDisposable : IDisposable 
    { 
     ArgumentValidate.NotNull(disposable, nameof(disposable)); 
     ArgumentValidate.NotNull(asyncFunc, nameof(asyncFunc)); 
     ArgumentValidate.IsEnumDefined(strategy, nameof(strategy)); 

     Exception mainException = null; 

     try 
     { 
      return await asyncFunc(disposable); 
     } 
     catch (Exception exception) 
     { 
      mainException = exception; 
      throw; 
     } 
     finally 
     { 
      try 
      { 
       disposable.Dispose(); 
      } 
      catch (Exception disposeException) 
      { 
       switch (strategy) 
       { 
        case DisposeExceptionStrategy.Propagate: 
         throw; 

        case DisposeExceptionStrategy.Swallow: 
         break; // swallow exception 

        case DisposeExceptionStrategy.Subjugate: 
         if (mainException == null) 
          throw; 
         break; // otherwise swallow exception 

        case DisposeExceptionStrategy.AggregateMultiple: 
         if (mainException != null) 
          throw new AggregateException(mainException, disposeException); 
         throw; 

        case DisposeExceptionStrategy.AggregateAlways: 
         if (mainException != null) 
          throw new AggregateException(mainException, disposeException); 
         throw new AggregateException(disposeException); 
       } 
      } 

      if (mainException != null && strategy == DisposeExceptionStrategy.AggregateAlways) 
       throw new AggregateException(mainException); 
     } 
    } 
}