Należy wdrożyć IDisposable.Dispose (), aby nigdy nie rzucał?

The ramowe wytyczne projektowe (2^nd ed) ma to postać (§9.4.1):

Unikaj rzucania wyjątku z wewnątrz Dispose (bool) z wyjątkiem pod krytycznym sytuacje, w których proces zawierający został uszkodzony (przecieki, niespójność wspólne państwo itp.).

Komentarz [Edit]:

• są wytyczne, nie twarde zasady. I to jest "unikaj", A Nie "Nie" wytyczna. Jak zaznaczono (w komentarzach) ramy łamie to (i inne) wytyczne w miejscach. Sztuką jest wiedzieć, kiedy złamać wytyczne. To, pod wieloma względami, jest różnicą między czeladnikiem a mistrzem.
• jeśli jakaś część sprzątania może się nie udać, powinna podać metodę Close, która wyrzuci wyjątki, aby wywołujący mógł je obsłużyć.
• jeśli postępujesz zgodnie ze wzorem dispose (i powinieneś być, jeśli Typ bezpośrednio zawiera jakiś niezarządzany zasób), to Dispose(bool) może być wywołany z finalizera, rzucając z finalizer jest złym pomysłem i blokuje inne obiekty przed finalizacją.

Mój pogląd: wyjątkami od Dispose powinny być tylko te, jak w wytycznej, które są na tyle katastrofalne, że nie jest możliwa dalsza wiarygodna funkcja z obecnego procesu.

Dispose powinien być zaprojektowany, aby wykonać swój cel, pozbywając się obiektu. To zadanie jest bezpieczne i nie wyrzuca WYJĄTKÓW przez większość czasu . Jeśli widzisz siebie rzucającego wyjątki z Dispose, prawdopodobnie powinieneś pomyśleć dwa razy, aby zobaczyć, czy robisz za dużo rzeczy w nim. Poza tym myślę, że Dispose powinny być traktowane jak wszystkie inne metody: handle jeśli możesz coś z tym zrobić, let it bubble jeśli nie możesz.

EDIT: dla podanego przykładu napisałbym kod tak, aby mój kod nie powoduje wyjątku, ale czyszczenie TcpClient w górę może spowodować wyjątek, który powinien być poprawny, aby propagować moim zdaniem (lub obsługiwać i rethrow jako bardziej ogólny wyjątek, tak jak każda metoda):

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

Jednak, tak jak każda metoda, jeśli wiesz, że tcpClient.Close() może rzucić wyjątek, który powinien być ignorowany (nie ma znaczenia) lub powinien być reprezentowany przez inny obiekt wyjątku, możesz chcieć go złapać.

Uwolnienie zasobów powinno być" bezpieczną " operacją - w końcu jak mogę odzyskać zasoby po tym, jak nie mogę zwolnić zasobów? więc rzucanie wyjątku od Dispose po prostu nie ma sensu.

Jednakże, jeśli odkryję wewnątrz Dispose, że stan programu jest uszkodzony, lepiej wyrzucić wyjątek, niż go połknąć, lepiej zmiażdżyć teraz, a następnie kontynuować działanie i produkować nieprawidłowe wyniki.

Szkoda, że Microsoft nie podał parametru Exception do Dispose, z zamiarem, że zostanie on zawinięty jako InnerException w przypadku, gdy sam dispose wyrzuci wyjątek. Aby być pewnym, efektywne użycie takiego parametru wymagałoby użycia bloku exception-filter, którego C# nie obsługuje, ale być może istnienie takiego parametru mogłoby zmotywować projektantów C# do udostępnienia takiej funkcji? Jedną miłą odmianą chciałbym zobaczyć byłoby dodanie wyjątku "parametr" do bloku finalnego, np.

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

Które zachowywałyby się jak normalny blok, z wyjątkiem tego, że 'ex' byłoby null/Nothing, gdyby 'Try' zakończyło się zakończeniem, lub utrzymywałoby wyrzucony wyjątek, gdyby nie to. Szkoda, że nie ma sposobu, aby istniejący kod używał takiej funkcji.

Prawdopodobnie użyłbym logowania, aby uchwycić szczegóły dotyczące pierwszego wyjątku, a następnie zezwolić na podniesienie drugiego wyjątku.

Istnieją różne strategie propagowania lub połykania WYJĄTKÓW z metody Dispose, prawdopodobnie w oparciu o to, czy nierozwiązany wyjątek został również wyrzucony z Głównej Logiki. Najlepszym rozwiązaniem byłoby pozostawienie decyzji rozmówcy, w zależności od jego specyficznych wymagań. Zaimplementowałem metodę generic extension, która to robi, oferując:

• domyślna using semantyka propagacji Dispose WYJĄTKÓW
• sugestia marca Gravella of always swallowing Dispose exceptions
• alternatywa maxyfc tylko Połykanie Dispose WYJĄTKÓW, gdy istnieje wyjątek od głównej logiki, który w przeciwnym razie zostałby utracony
• podejście Daniela Chambersa owijania wielu WYJĄTKÓW w AggregateException
• podobne podejście, które zawsze owija wszystkie wyjątki w AggregateException (Jak Task.Wait robi)

To jest moja metoda rozszerzenia:

/// <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);
        }
    }
}

Są to zaimplementowane strategie:

/// <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,
}

Użycie próbki:

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

Update: jeśli chcesz obsługiwać delegatów, które zwracają wartości i / lub są asynchroniczne, możesz użyć tych przeciążeń:

/// <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);
        }
    }
}

Oto sposób na całkiem czyste przechwycenie wszelkich WYJĄTKÓW rzuconych przez zawartość using lub Dispose.

Oryginalny kod:

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

Oto kod, który rzuci jeśli codeInUsing() rzuci lub foo.Dispose() rzuci lub oba rzuci, i pokaże pierwszy wyjątek (czasami zawinięty jako Innerexpress, w zależności):

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

Oto kod do wdrożenia tego. Mam go ustawiony tak, że tylko działa zgodnie z opisem, gdy debugger nie jest dołączony, ponieważ gdy debugger jest dołączony, bardziej martwię się, że złamie się we właściwym miejscu na pierwszym wyjątku. Można modyfikować w razie potrzeby.

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();
            }
        }
    }
}