Maneira correta de lidar com exceções no AsyncDispose

20

Durante a mudança para o novo .NET Core 3 IAsynsDisposable, deparei-me com o seguinte problema.

O núcleo do problema: se DisposeAsynclança uma exceção, essa exceção oculta todas as exceções lançadas 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");
    }
}

O que está sendo pego é a AsyncDisposeexceção-se lançada, e a exceção de dentro await usingsomente se AsyncDisposenão for lançada .

No entanto, eu preferiria o contrário: obter a exceção do await usingbloco, se possível, e DisposeAsync-exception somente se o await usingbloco for concluído com êxito.

Justificativa: imagine que minha classe Dfuncione com alguns recursos de rede e assine algumas notificações remotas. O código interno await usingpode fazer algo errado e falhar no canal de comunicação; depois disso, o código em Dispose, que tenta fechar normalmente a comunicação (por exemplo, cancelar a assinatura das notificações) também falharia. Mas a primeira exceção fornece informações reais sobre o problema, e a segunda é apenas um problema secundário.

No outro caso, quando a parte principal foi executada e a eliminação falhou, o problema real está dentro DisposeAsync, portanto a exceção DisposeAsyncé a relevante. Isso significa que apenas suprimir todas as exceções internas DisposeAsyncnão deve ser uma boa ideia.


Eu sei que existe o mesmo problema com o caso não assíncrono: a exceção finallysubstitui a exceção try, é por isso que não é recomendável lançar Dispose(). Mas, com as classes de acesso à rede, suprimir exceções nos métodos de fechamento não parece bom.


É possível solucionar o problema com o seguinte auxiliar:

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 { }
        }
    }
}

e use-o como

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

que é meio feio (e não permite coisas como retornos antecipados dentro do bloco using).

Existe uma solução boa e canônica, await usingse possível? Minha pesquisa na internet não encontrou nem discutir esse problema.

Vlad
fonte
11
" Mas, com as classes de acesso à rede, suprimir exceções nos métodos de fechamento não parece bom " - acho que a maioria das classes BLC em rede tem um Closemétodo separado por esse motivo. Provavelmente, é aconselhável fazer o mesmo: CloseAsynctentativas de fechar bem as coisas e fracassar. DisposeAsyncapenas faz o seu melhor e falha silenciosamente.
canton7
@ canton7: ​​Bem, ter um CloseAsyncmeio separado significa que preciso tomar precauções extras para fazê-lo funcionar. Se eu apenas colocá-lo no final do usingbloco, ele será ignorado nos retornos antecipados etc. (é isso que gostaríamos que acontecesse) e exceções (é isso que gostaríamos que acontecesse). Mas a ideia parece promissora.
Vlad
Há uma razão pela qual muitos padrões de codificação proíbem retornos antecipados :) Nos locais em que a rede está envolvida, ser um pouco explícito não é algo ruim para a IMO. Disposesempre foi "As coisas podem ter dado errado: basta fazer o seu melhor para melhorar a situação, mas não piorar", e não vejo por que AsyncDisposedeveria ser diferente.
canton7
@ canton7: ​​Bem, em um idioma com exceções, toda declaração pode ser um retorno antecipado: - \
Vlad
Certo, mas esses serão excepcionais . Nesse caso, DisposeAsyncfazer o melhor para arrumar, mas não jogar é a coisa certa a fazer. Você estava falando sobre retornos antecipados intencionais , nos quais um retorno antecipado intencional pode ignorar erroneamente uma chamada para CloseAsync: esses são os proibidos por muitos padrões de codificação.
canton7

Respostas:

3

Há exceções que você deseja exibir (interrompa a solicitação atual ou interrompa o processo) e há exceções que seu projeto espera que ocorram às vezes e você possa lidar com elas (por exemplo, tente novamente e continue).

Mas a distinção entre esses dois tipos depende do chamador final do código - este é o ponto inteiro das exceções, para deixar a decisão por conta do chamador.

Às vezes, o chamador dará prioridade maior à superfície da exceção do bloco de código original e, às vezes, à exceção do Dispose. Não há regra geral para decidir qual deve ter prioridade. O CLR é pelo menos consistente (como você observou) entre o comportamento de sincronização e não-assíncrono.

Talvez seja lamentável que agora tenhamos que AggregateExceptionrepresentar várias exceções, não possa ser adaptado para resolver isso. ou seja, se uma exceção já estiver em andamento e outra for lançada, elas serão combinadas em uma AggregateException. O catchmecanismo pode ser modificado para que, se você escrever catch (MyException), ele capture qualquer um AggregateExceptionque inclua uma exceção do tipo MyException. Porém, existem várias outras complicações decorrentes dessa idéia e provavelmente é muito arriscado modificar algo tão fundamental agora.

Você pode melhorar seu UsingAsyncsuporte ao retorno antecipado de um 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
fonte
Então, eu entendi correto: sua idéia é que, em alguns casos, apenas o padrão await usingpode ser usado (é aqui que o DisposeAsync não será lançado em casos não fatais) e um auxiliar como UsingAsyncé mais apropriado (se é provável que DisposeAsync seja lançado) ? (Claro, eu preciso modificar UsingAsyncde modo que ele não cegamente pegar tudo, mas só não fatal (e não-boneheaded no uso de Eric Lippert ).)
Vlad
@ Vlad sim - a abordagem correta é totalmente dependente do contexto. Observe também que UsingAsync não pode ser gravado uma vez para usar alguma categorização globalmente verdadeira de tipos de exceção, dependendo se eles devem ser capturados ou não. Novamente, esta é uma decisão a ser tomada de maneira diferente, dependendo da situação. Quando Eric Lippert fala dessas categorias, elas não são fatos intrínsecos sobre os tipos de exceção. A categoria por tipo de exceção depende do seu design. Às vezes, uma IOException é esperada por design, às vezes não.
Daniel Earwicker
4

Talvez você já entenda por que isso acontece, mas vale a pena explicar. Esse comportamento não é específico para await using. Isso aconteceria com um usingbloco simples também. Então, enquanto eu digo Dispose()aqui, tudo se aplica DisposeAsync()também.

Um usingbloco é apenas um açúcar sintático para um try/ finallybloco, como diz a seção de comentários da documentação . O que você vê acontece porque o finallybloco sempre é executado, mesmo após uma exceção. Portanto, se uma exceção ocorrer e não houver catchbloco, a exceção será colocada em espera até que o finallybloco seja executado e a exceção será lançada. Mas se ocorrer uma exceção finally, você nunca verá a exceção antiga.

Você pode ver isso com este exemplo:

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

Não importa se é chamado Dispose()ou não DisposeAsync()dentro do finally. O comportamento é o mesmo.

Meu primeiro pensamento é: não jogue Dispose(). Mas depois de revisar parte do código da Microsoft, acho que depende.

Veja a implementação deles FileStream, por exemplo. Tanto o Dispose()método síncrono , como DisposeAsync()pode realmente lançar exceções. O síncrono Dispose()faz ignorar algumas exceções intencionalmente, mas não todos.

Mas acho importante levar em conta a natureza da sua turma. Em um FileStream, por exemplo, Dispose()liberará o buffer no sistema de arquivos. Essa é uma tarefa muito importante e você precisa saber se isso falhou . Você não pode simplesmente ignorar isso.

No entanto, em outros tipos de objetos, quando você chama Dispose(), você realmente não tem mais uso para o objeto. Ligar Dispose()realmente significa "esse objeto está morto para mim". Talvez ele limpe alguma memória alocada, mas falhar não afeta a operação do seu aplicativo de forma alguma. Nesse caso, você pode decidir ignorar a exceção dentro do seu Dispose().

Mas, em qualquer caso, se você quiser distinguir entre uma exceção dentro usingou uma exceção que veio Dispose(), precisará de um bloco try/ catchdentro e fora do seu usingbloco:

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
}

Ou você simplesmente não pode usar using. Escreva um bloco try/ catch/ finallyvocê mesmo, onde você encontra alguma exceção em 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
fonte
3
Btw, source.dot.net (.NET Core) / referencesource.microsoft.com (.NET Framework) é muito mais fácil de navegar do que GitHub
canton7
Obrigado pela sua resposta! Eu sei qual é o verdadeiro motivo (mencionei try / finalmente e síncrono na pergunta). Agora sobre sua proposta. Uma parte catch interna do usingbloco não ajudaria, porque geralmente o tratamento de exceções é feito em algum lugar longe do usingpróprio bloco, portanto, o manuseio dentro do bloco usinggeralmente não é muito viável. Sobre o uso de não using- é realmente melhor que a solução proposta?
Vlad
2
@ canton7 Awesome! Eu conhecia o sourcesource.microsoft.com , mas não sabia que havia um equivalente para o .NET Core. Obrigado!
Gabriel Luci
@ Vlad "Melhor" é algo que só você pode responder. Sei que estava lendo o código de outra pessoa, eu preferiria ver try/ catch/ finallybloquear, pois ficaria imediatamente claro o que está fazendo sem precisar ler o que AsyncUsingestá fazendo. Você também tem a opção de fazer um retorno antecipado. Também haverá um custo adicional de CPU para o seu AwaitUsing. Seria pequeno, mas está lá.
Gabriel Luci
2
@PauloMorgado Isso significa apenas que Dispose()não deve ser lançado porque é chamado mais de uma vez. As implementações da própria Microsoft podem gerar exceções, e por boas razões, como mostrei nesta resposta. No entanto, concordo que você deve evitá-lo se possível, pois normalmente ninguém esperaria que ele fosse lançado.
Gabriel Luci
4

O uso é efetivamente o Código de Tratamento de Exceções (sintaxe sugar para try ... finalmente ... Dispose ()).

Se o seu código de manipulação de exceções estiver lançando Exceções, algo será eliminado.

O que quer que tenha acontecido para levá-lo até lá, não importa mais. O código de manipulação de exceção com defeito oculta todas as exceções possíveis, de uma maneira ou de outra. O código de manipulação de exceção deve ser corrigido, com prioridade absoluta. Sem isso, você nunca obtém dados de depuração suficientes para o problema real. Vejo que isso é feito com extrema frequência, muitas vezes. É tão fácil errar quanto lidar com ponteiros nus. Muitas vezes, existem dois artigos sobre a temática que eu vinculo, que podem ajudá-lo com quaisquer conceitos errôneos de design subjacentes:

Dependendo da classificação de exceção, é isso que você precisa fazer se seu código de manipulação / difusão de exceções gerar uma exceção:

Para Fatal, Boneheaded e Vexing, a solução é a mesma.

Exceções exógenas, devem ser evitadas mesmo a um custo sério. Há uma razão pela qual ainda usamos arquivos de log em vez de registrar bancos de dados para registrar exceções - as operações do banco de dados são uma maneira de propender a problemas exógenos. Os arquivos de log são o único caso, onde eu nem me importo se você mantiver o identificador de arquivo aberto durante todo o tempo de execução.

Se você tiver que fechar uma conexão, não se preocupe muito com a outra extremidade. Manuseie-o como o UDP: "Vou enviar as informações, mas não me importo se o outro lado conseguir." A eliminação é sobre a limpeza de recursos no lado / lado do cliente em que você está trabalhando.

Eu posso tentar notificá-los. Mas limpar as coisas no lado do servidor / FS? É por isso que o tempo limite e o tratamento de exceções são responsáveis.

Christopher
fonte
Portanto, sua proposta se resume a suprimir exceções no fechamento de conexão, certo?
Vlad
@Vlad Exogenous ones? Certo. Dipose / Finalizador estão lá para limpar por conta própria. Provavelmente, ao fechar a Instância Conneciton devido a uma exceção, você o fará porque não terá mais uma conexão ativa com elas. E que ponto haveria na obtenção de uma exceção "Sem conexão" ao manipular a exceção anterior "Sem conexão"? Você envia um único "Ei, estou fechando esta conexão", onde você ignora todas as exceções exógenas ou mesmo se elas se aproximam do alvo. Já as implementações padrão do Dispose já fazem isso.
Christopher
@ Vlad: Eu lembrei que existem muitas coisas que você nunca deve lançar exceções (exceto as corretas, fatais). Os inicializadores de tipo estão no topo da lista. Dispose também é um deles: "Para ajudar a garantir que os recursos sejam sempre limpos adequadamente, um método Dispose deve ser chamado várias vezes sem gerar uma exceção". docs.microsoft.com/en-us/dotnet/standard/garbage-collection/…
Christopher
@ Vlad A chance de exceções fatais? Nós sempre temos que arriscar aqueles e nunca devemos lidar com eles além de "chamar Dispose". E realmente não deve fazer nada com isso. Na verdade, eles não são mencionados em nenhuma documentação. | Exceções desossadas? Sempre conserte-os. | Exceções irritantes são as principais candidatas a engolir / manipular, como no TryParse () | Exógena? Também deve sempre ser manuseado. Muitas vezes, você também deseja informar o usuário sobre eles e registrá-los. Mas, caso contrário, não vale a pena acabar com o processo.
Christopher
@ Vlad Eu procurei SqlConnection.Dispose (). Ele nem se importa em enviar algo ao servidor sobre o término da conexão. Algo ainda pode acontecer como resultado de NativeMethods.UnmapViewOfFile();e NativeMethods.CloseHandle(). Mas esses são importados de fora. Não há verificação de qualquer valor de retorno ou qualquer outra coisa que possa ser usada para obter uma exceção do .NET adequada em torno do que esses dois possam encontrar. Então, eu estou fortemente convencido, SqlConnection.Dispose (bool) simplesmente não se importa. | Fechar é muito melhor, na verdade informando o servidor. Antes de ligar, descarte.
Christopher
1

Você pode tentar usar AggregateException e modificar seu código da seguinte maneira:

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
fonte