Manera adecuada de manejar las excepciones en AsyncDispose

20

Durante el cambio a los nuevos .NET Core 3 IAsynsDisposable, me encontré con el siguiente problema.

El núcleo del problema: si DisposeAsyncarroja una excepción, esta excepción oculta cualquier excepción lanzada dentro de await using-block.

class Program 
{
    static async Task Main()
    {
        try
        {
            await using (var d = new D())
            {
                throw new ArgumentException("I'm inside using");
            }
        }
        catch (Exception e)
        {
            Console.WriteLine(e.Message); // prints I'm inside dispose
        }
    }
}

class D : IAsyncDisposable
{
    public async ValueTask DisposeAsync()
    {
        await Task.Delay(1);
        throw new Exception("I'm inside dispose");
    }
}

Lo que queda atrapado es la AsyncDisposeexcepción-si se lanza, y la excepción desde adentro await usingsolo si AsyncDisposeno se lanza.

Sin embargo, preferiría lo contrario: obtener la excepción del await usingbloque si es posible, y DisposeAsync-excepción solo si el await usingbloque terminó con éxito.

Justificación: Imagine que mi clase Dtrabaja con algunos recursos de red y se suscribe a algunas notificaciones remotas. El código interno await usingpuede hacer algo mal y fallar el canal de comunicación, luego el código en Dispose que intenta cerrar la comunicación con gracia (por ejemplo, cancelar la suscripción a las notificaciones) también fallará. Pero la primera excepción me da la información real sobre el problema, y ​​la segunda es solo un problema secundario.

En el otro caso, cuando la parte principal se ejecutó y la eliminación falló, el verdadero problema está dentro DisposeAsync, por lo que la excepción DisposeAsynces la relevante. Esto significa que suprimir todas las excepciones en el interior DisposeAsyncno debería ser una buena idea.


Sé que existe el mismo problema con el caso no asíncrono: la excepción en finallyanula la excepción en try, por eso no se recomienda incluir Dispose(). Pero con las clases de acceso a la red que suprimen las excepciones en los métodos de cierre no se ve nada bien.


Es posible solucionar el problema con el siguiente ayudante:

static class AsyncTools
{
    public static async Task UsingAsync<T>(this T disposable, Func<T, Task> task)
            where T : IAsyncDisposable
    {
        bool trySucceeded = false;
        try
        {
            await task(disposable);
            trySucceeded = true;
        }
        finally
        {
            if (trySucceeded)
                await disposable.DisposeAsync();
            else // must suppress exceptions
                try { await disposable.DisposeAsync(); } catch { }
        }
    }
}

y úsalo como

await new D().UsingAsync(d =>
{
    throw new ArgumentException("I'm inside using");
});

lo cual es un poco feo (y no permite cosas como retornos tempranos dentro del bloque de uso).

¿Existe una buena solución canónica, await usingsi es posible? Mi búsqueda en internet no encontró ni siquiera discutir este problema.

Vlad
fuente
1
" Pero con las clases de acceso a la red, la supresión de excepciones en los métodos de cierre no se ve bien en absoluto ". Creo que la mayoría de las clases BLC en red tienen un Closemétodo separado por esta misma razón. Probablemente sea aconsejable hacer lo mismo: CloseAsyncintenta cerrar las cosas muy bien y arroja el fracaso. DisposeAsyncsimplemente hace lo mejor y falla en silencio.
canton7
@ canton7: ​​Bueno, tener un CloseAsyncmedio separado significa que debo tomar precauciones adicionales para que funcione. Si lo pongo al final de using-block, se omitirá en los primeros retornos, etc. (esto es lo que queremos que suceda) y excepciones (esto es lo que queremos que suceda). Pero la idea parece prometedora.
Vlad
Hay una razón por la que muchos estándares de codificación prohíben los retornos tempranos :) Cuando se trata de redes, ser un poco explícito no es malo en mi opinión. Disposesiempre ha sido "Las cosas podrían haber salido mal: solo haz tu mejor esfuerzo para mejorar la situación, pero no la empeores", y no veo por qué AsyncDisposedebería ser diferente.
canton7
@ canton7: ​​Bueno, en un lenguaje con excepciones, cada declaración puede ser un retorno temprano: - \
Vlad
Correcto, pero serán excepcionales . En ese caso, hacer DisposeAsynclo mejor para ordenar pero no tirar es lo correcto. Estaba hablando de devoluciones anticipadas intencionales , donde una devolución anticipada intencional podría omitir por error una llamada a CloseAsync: esas son las prohibidas por muchos estándares de codificación.
canton7

Respuestas:

3

Hay excepciones que desea mostrar (interrumpir la solicitud actual o reducir el proceso), y hay excepciones que su diseño espera que ocurran a veces y puede manejarlas (por ejemplo, vuelva a intentarlo y continúe).

Pero distinguir entre estos dos tipos depende de la persona que realiza la última llamada del código; este es el punto principal de las excepciones, dejar la decisión a la persona que llama.

A veces, la persona que llama dará mayor prioridad a la aparición de la excepción del bloque de código original, y a veces la excepción del Dispose. No existe una regla general para decidir cuál debe tener prioridad. El CLR es al menos consistente (como ha notado) entre el comportamiento sincronizado y el no asíncrono.

Tal vez sea desafortunado que ahora tengamos AggregateExceptionque representar múltiples excepciones, no se puede adaptar para resolver esto. es decir, si una excepción ya está en vuelo y se lanza otra, se combinan en un AggregateException. El catchmecanismo podría modificarse para que, si escribe catch (MyException), capture cualquiera AggregateExceptionque incluya una excepción de tipo MyException. Sin embargo, hay varias otras complicaciones derivadas de esta idea, y probablemente sea demasiado arriesgado modificar algo tan fundamental ahora.

Podría mejorar su UsingAsyncpara respaldar la devolución anticipada de un valor:

public static async Task<R> UsingAsync<T, R>(this T disposable, Func<T, Task<R>> task)
        where T : IAsyncDisposable
{
    bool trySucceeded = false;
    R result;
    try
    {
        result = await task(disposable);
        trySucceeded = true;
    }
    finally
    {
        if (trySucceeded)
            await disposable.DisposeAsync();
        else // must suppress exceptions
            try { await disposable.DisposeAsync(); } catch { }
    }
    return result;
}
Daniel Earwicker
fuente
Entonces, entiendo lo correcto: su idea es que en algunos casos solo await usingse puede usar estándar (aquí es donde DisposeAsync no arrojará un caso no fatal), y un ayudante como UsingAsynces más apropiado (si es probable que DisposeAsync arroje) ? (Por supuesto, tendría que modificar UsingAsyncpara que no capte todo a ciegas, pero solo no sea fatal (y no descabellado en el uso de Eric Lippert ).)
Vlad
@Vlad sí, el enfoque correcto depende totalmente del contexto. También tenga en cuenta que UsingAsync no se puede escribir una vez para usar alguna categorización globalmente verdadera de los tipos de excepción de acuerdo con si deben capturarse o no. Nuevamente, esta es una decisión que debe tomarse de manera diferente según la situación. Cuando Eric Lippert habla de esas categorías, no son hechos intrínsecos sobre los tipos de excepción. La categoría por tipo de excepción depende de su diseño. A veces se espera una IOException por diseño, a veces no.
Daniel Earwicker
4

Quizás ya entiendas por qué sucede esto, pero vale la pena explicarlo. Este comportamiento no es específico para await using. También sucedería con un usingbloque simple . Entonces, mientras digo Dispose()aquí, todo se aplica DisposeAsync()también.

Un usingbloque es solo azúcar sintáctico para un try/ finallybloque, como dice la sección de comentarios de la documentación . Lo que ves sucede porque el finallybloque siempre se ejecuta, incluso después de una excepción. Entonces, si ocurre una excepción y no hay catchbloque, la excepción se pone en espera hasta finallyque se ejecuta el bloque y luego se lanza la excepción. Pero si ocurre una excepción finally, nunca verá la excepción anterior.

Puedes ver esto con este ejemplo:

try {
    throw new Exception("Inside try");
} finally {
    throw new Exception("Inside finally");
}

No importa si Dispose()o DisposeAsync()se llama dentro de la finally. El comportamiento es el mismo.

Mi primer pensamiento es: no tirar Dispose(). Pero después de revisar algunos de los códigos propios de Microsoft, creo que depende.

Eche un vistazo a su implementación de FileStream, por ejemplo. Tanto el Dispose()método sincrónico , y en DisposeAsync()realidad puede lanzar excepciones. El síncrona Dispose()hace caso omiso de algunas excepciones intencionalmente, pero no todos.

Pero creo que es importante tener en cuenta la naturaleza de tu clase. En un FileStream, por ejemplo, Dispose()vaciará el búfer al sistema de archivos. Esa es una tarea muy importante y necesita saber si eso falló . No puedes ignorar eso.

Sin embargo, en otros tipos de objetos, cuando llamas Dispose(), ya no tienes uso para el objeto. Llamar Dispose()realmente solo significa "este objeto está muerto para mí". Tal vez limpia parte de la memoria asignada, pero la falla no afecta el funcionamiento de su aplicación de ninguna manera. En ese caso, puede decidir ignorar la excepción dentro de su Dispose().

Pero en cualquier caso, si desea distinguir entre una excepción dentro usingo una excepción que proviene Dispose(), entonces necesita un try/ catchbloque tanto dentro como fuera de su usingbloque:

try {
    await using (var d = new D())
    {
        try
        {
            throw new ArgumentException("I'm inside using");
        }
        catch (Exception e)
        {
            Console.WriteLine(e.Message); // prints I'm inside using
        }
    }
} catch (Exception e) {
    Console.WriteLine(e.Message); // prints I'm inside dispose
}

O simplemente no podrías usar using. Escriba a try/ catch/ finallyblock usted mismo, donde detecte cualquier excepción en finally:

var d = new D();
try
{
    throw new ArgumentException("I'm inside try");
}
catch (Exception e)
{
    Console.WriteLine(e.Message); // prints I'm inside try
}
finally
{
    try
    {
        if (D != null) await D.DisposeAsync();
    }
    catch (Exception e)
    {
        Console.WriteLine(e.Message); // prints I'm inside dispose
    }
}
Gabriel Luci
fuente
3
Por cierto , source.dot.net (.NET Core) / referencesource.microsoft.com (.NET Framework) es mucho más fácil de navegar que GitHub
canton7
¡Gracias por su respuesta! Sé cuál es la verdadera razón (mencioné probar / finalmente y el caso sincrónico en la pregunta). Ahora sobre tu propuesta. Un catch interior del usingbloque no ayudaría porque, por lo general, el manejo de excepciones se realiza en algún lugar lejos del usingbloque en sí, por lo que no usinges muy práctico manejarlo dentro . Sobre el uso de no using, ¿es realmente mejor que la solución propuesta?
Vlad
2
@ canton7 ¡Impresionante! Estaba al tanto de referencesource.microsoft.com , pero no sabía que había un equivalente para .NET Core. ¡Gracias!
Gabriel Luci el
@Vlad "Mejor" es algo que solo tú puedes responder. Sé que si estaba leyendo el código de otra persona, preferiría ver try/ catch/ finallybloquear ya que sería claro de inmediato lo que está haciendo sin tener que ir a leer lo que AsyncUsingestá haciendo. También tiene la opción de hacer un regreso temprano. También habrá un costo adicional de CPU para su AwaitUsing. Sería pequeño, pero está ahí.
Gabriel Luci el
2
@PauloMorgado Eso solo significa que Dispose()no se debe tirar porque se llama más de una vez. Las propias implementaciones de Microsoft pueden arrojar excepciones, y por una buena razón, como he mostrado en esta respuesta. Sin embargo, estoy de acuerdo en que debe evitarlo si es posible, ya que nadie esperaría que se lance normalmente.
Gabriel Luci el
4

usar es efectivamente el Código de manejo de excepciones (sintaxis de azúcar para probar ... finalmente ... Dispose ()).

Si su código de manejo de excepciones arroja Excepciones, algo está realmente roto.

Cualquier otra cosa que te haya metido allí, ya no importa. El código de manejo de excepciones defectuosas ocultará todas las excepciones posibles, de una forma u otra. El código de manejo de excepciones debe ser fijo, eso tiene prioridad absoluta. Sin eso, nunca obtienes suficientes datos de depuración para el problema real. Veo que se hace mal extremadamente a menudo. Es tan fácil equivocarse como manejar punteros desnudos. Muy a menudo, hay dos artículos sobre el tema temático que enlace, que pueden ayudarlo con cualquier concepción errónea subyacente:

Dependiendo de la clasificación de excepciones, esto es lo que debe hacer si su código de manejo de excepciones / dipose produce una excepción:

Para Fatal, Boneheaded y Vexing, la solución es la misma.

Excepciones exógenas, deben evitarse incluso a un costo serio. Hay una razón por la que todavía utilizamos archivos de registro en lugar de bases de datos de registro para registrar excepciones: las operaciones de base de datos DB son solo una forma de ser propensos a tener problemas exógenos. Los archivos de registro son el único caso, donde ni siquiera me importa si mantiene el controlador de archivo abierto durante todo el tiempo de ejecución.

Si tienes que cerrar una conexión, no te preocupes demasiado por el otro extremo. Manéjelo como lo hace UDP: "Enviaré la información, pero no me importa si la otra parte la obtiene". Desechar es limpiar recursos del lado del cliente en el que está trabajando.

Puedo intentar notificarles. ¿Pero limpiar cosas en el lado del servidor / FS? De eso son responsables sus tiempos de espera y su manejo de excepciones.

Christopher
fuente
Entonces, su propuesta se reduce a suprimir excepciones en el cierre de la conexión, ¿verdad?
Vlad
@Vlad Exógenas? Por supuesto. Dipose / Finalizer están ahí para limpiar por su propio lado. Lo más probable es que al cerrar la instancia de Conneciton debido a una excepción, lo haga porque ya no tiene una conexión activa con ellos. ¿Y qué punto tendría obtener una excepción "Sin conexión" mientras se maneja la excepción anterior "Sin conexión"? Envía un solo "Yo, estoy cerrando esta conexión" donde ignora todas las Excepciones exógenas o incluso si se acerca al objetivo. Afaik, las implementaciones predeterminadas de Dispose ya lo hacen.
Christopher
@Vlad: Recordé que hay un montón de cosas de las que nunca se supone que arrojes excepciones (excepto Coruse Fatal). Los iniciadores de tipo están muy arriba en la lista. Dispose también es uno de esos: "Para ayudar a garantizar que los recursos siempre se limpien de manera adecuada, un método Dispose debe ser invocable varias veces sin generar una excepción". docs.microsoft.com/en-us/dotnet/standard/garbage-collection/…
Christopher
@Vlad La posibilidad de excepciones fatales? Siempre tenemos que arriesgarlos y nunca debemos manejarlos más allá de "llamar a Eliminar". Y realmente no debería hacer nada con ellos. De hecho, no se mencionan en ninguna documentación. El | ¿Excepciones deshonestas? Siempre arreglarlos. El | Las excepciones molestas son los principales candidatos para la deglución / manipulación, como en TryParse () | Exógeno? También siempre debe ser manipulado. A menudo, también desea informar al usuario sobre ellos y registrarlos. Pero de lo contrario, no vale la pena matar tu proceso.
Christopher
@Vlad Busqué SqlConnection.Dispose (). Ni siquiera le importa enviar nada al Servidor acerca de que la conexión ha terminado. Algo podría suceder como resultado de NativeMethods.UnmapViewOfFile();y NativeMethods.CloseHandle(). Pero esos son importados desde el exterior. No se verifica ningún valor de retorno o cualquier otra cosa que pueda usarse para obtener una Excepción .NET adecuada en torno a lo que puedan encontrar esos dos. Así que estoy asumiendo firmemente que SqlConnection.Dispose (bool) simplemente no le importa. El | Cerrar es mucho mejor, en realidad le dice al servidor. Antes de que llame disponga.
Christopher
1

Puede intentar usar AggregateException y modificar su código de la siguiente manera:

class Program 
{
    static async Task Main()
    {
        try
        {
            await using (var d = new D())
            {
                throw new ArgumentException("I'm inside using");
            }
        }
        catch (AggregateException ex)
        {
            ex.Handle(inner =>
            {
                if (inner is Exception)
                {
                    Console.WriteLine(e.Message);
                }
            });
        }
        catch (Exception e)
        {
            Console.WriteLine(e.Message);
        }
    }
}

class D : IAsyncDisposable
{
    public async ValueTask DisposeAsync()
    {
        await Task.Delay(1);
        throw new Exception("I'm inside dispose");
    }
}

https://docs.microsoft.com/ru-ru/dotnet/api/system.aggregateexception?view=netframework-4.8

https://docs.microsoft.com/ru-ru/dotnet/standard/parallel-programming/exception-handling-task-parallel-library

GDI89
fuente