Manière appropriée de traiter les exceptions dans AsyncDispose

20

Lors du passage aux nouveaux .NET Core 3 IAsynsDisposable, je suis tombé sur le problème suivant.

Le cœur du problème: si DisposeAsynclève une exception, cette exception cache toutes les exceptions await usinglevées à l' intérieur de -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");
    }
}

Ce qui se fait attraper, c'est l' AsyncDisposeexception-si elle est lancée, et l'exception de l'intérieur await usinguniquement si AsyncDisposeelle ne lance pas.

Je préfère cependant l'inverse: obtenir l'exception du await usingbloc si possible, et DisposeAsync-exception uniquement si le await usingbloc s'est terminé avec succès.

Justification: Imaginez que ma classe Dfonctionne avec certaines ressources réseau et s'abonne à certaines notifications à distance. Le code à l'intérieur await usingpeut faire quelque chose de mal et échouer le canal de communication, après quoi le code dans Dispose qui essaie de fermer la communication avec élégance (par exemple, se désinscrire des notifications) échouera également. Mais la première exception me donne les vraies informations sur le problème, et la seconde n'est qu'un problème secondaire.

Dans l'autre cas, lorsque la partie principale a traversé et que l'élimination a échoué, le vrai problème est à l'intérieur DisposeAsync, donc l'exception de DisposeAsyncest pertinente. Cela signifie que la suppression de toutes les exceptions à l'intérieur DisposeAsyncne devrait pas être une bonne idée.


Je sais qu'il y a le même problème avec le cas non asynchrone: l'exception dans finallyremplace l'exception dans try, c'est pourquoi il n'est pas recommandé de le faire Dispose(). Mais avec les classes d'accès au réseau, la suppression des exceptions dans les méthodes de fermeture ne semble pas du tout bonne.


Il est possible de contourner le problème avec l'aide suivante:

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

et l'utiliser comme

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

ce qui est un peu moche (et interdit les choses comme les premiers retours à l'intérieur du bloc using).

Existe-t-il une bonne solution canonique, avec await usingsi possible? Ma recherche sur Internet n'a même pas permis de discuter de ce problème.

Vlad
la source
1
" Mais avec les classes d'accès au réseau, la suppression des exceptions dans les méthodes de fermeture ne semble pas du tout bonne " - je pense que la plupart des classes BLC en réseau ont une Closeméthode distincte pour cette raison. Il est probablement sage de faire de même: CloseAsynctente de bien fermer les choses et jette l'échec. DisposeAsyncfait de son mieux et échoue silencieusement.
canton7
@ canton7: ​​Eh bien, ayant un CloseAsyncmoyen séparé , je dois prendre des précautions supplémentaires pour le faire fonctionner. Si je le mets juste à la fin de using-block, il sera ignoré lors des premiers retours, etc. Mais l'idée semble prometteuse.
Vlad
Il y a une raison pour laquelle de nombreuses normes de codage interdisent les retours anticipés :) Lorsque le réseautage est impliqué, être un peu explicite n'est pas une mauvaise chose OMI. Disposea toujours été "Les choses ont peut-être mal tourné: faites simplement de votre mieux pour améliorer la situation, mais ne faites pas qu'aggraver", et je ne vois pas pourquoi cela AsyncDisposedevrait être différent.
canton7
@ canton7: ​​Eh bien, dans une langue avec des exceptions, chaque déclaration peut être un retour anticipé: - \
Vlad
D'accord, mais ce sera exceptionnel . Dans ce cas, DisposeAsyncfaire de son mieux pour ranger mais pas jeter est la bonne chose à faire. Vous parliez de retours anticipés intentionnels , où un retour anticipé intentionnel pourrait par erreur contourner un appel à CloseAsync: ce sont ceux qui sont interdits par de nombreuses normes de codage.
canton7

Réponses:

3

Il existe des exceptions que vous souhaitez faire apparaître (interrompre la demande en cours ou interrompre le processus), et il existe des exceptions qui, selon votre conception, se produiront parfois et vous pouvez les gérer (par exemple, réessayer et continuer).

Mais la distinction entre ces deux types appartient à l'appelant ultime du code - c'est tout l'intérêt des exceptions, de laisser la décision à l'appelant.

Parfois, l'appelant accordera une plus grande priorité à la détection de l'exception du bloc de code d'origine, et parfois de l'exception du Dispose. Il n'y a pas de règle générale pour décider laquelle devrait être prioritaire. Le CLR est au moins cohérent (comme vous l'avez noté) entre le comportement de synchronisation et non asynchrone.

Il est peut-être regrettable que nous devions maintenant AggregateExceptionreprésenter plusieurs exceptions, il ne peut pas être modifié pour résoudre ce problème. c'est-à-dire que si une exception est déjà en vol et qu'une autre est levée, elles sont combinées en un AggregateException. Le catchmécanisme pourrait être modifié de sorte que si vous écrivez, catch (MyException)il interceptera tout ce AggregateExceptionqui inclut une exception de type MyException. Il existe cependant diverses autres complications découlant de cette idée, et il est probablement trop risqué de modifier quelque chose de si fondamental maintenant.

Vous pouvez améliorer votre UsingAsyncpour prendre en charge le retour précoce d'une valeur:

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
la source
Donc, je comprends bien: votre idée est que dans certains cas, seul le standard await usingpeut être utilisé (c'est là que DisposeAsync ne lancera pas dans un cas non fatal), et un assistant comme UsingAsyncest plus approprié (si DisposeAsync est susceptible de lancer) ? (Bien sûr, je devrais le modifier UsingAsyncpour qu'il n'attrape pas tout aveuglément, mais seulement non fatal (et non sans tête dans l'utilisation d'Eric Lippert ).)
Vlad
@ Vlad oui - la bonne approche dépend totalement du contexte. Notez également que UsingAsync ne peut pas être écrit une fois pour utiliser une catégorisation globale des types d'exceptions selon qu'ils doivent être interceptés ou non. Encore une fois, c'est une décision à prendre différemment selon la situation. Quand Eric Lippert parle de ces catégories, il ne s'agit pas de faits intrinsèques sur les types d'exception. La catégorie par type d'exception dépend de votre conception. Parfois, une exception IOException est attendue par conception, parfois non.
Daniel Earwicker
4

Peut-être que vous comprenez déjà pourquoi cela se produit, mais cela vaut la peine d'être expliqué. Ce comportement n'est pas spécifique à await using. Cela arriverait aussi avec un usingbloc simple . Donc, bien que je dis Dispose()ici, tout cela s'applique DisposeAsync()aussi.

Un usingbloc est juste du sucre syntaxique pour un bloc try/ finally, comme le dit la section des remarques de la documentation . Ce que vous voyez se produit car le finallybloc s'exécute toujours , même après une exception. Donc, si une exception se produit et qu'il n'y a pas de catchbloc, l'exception est mise en attente jusqu'à ce que le finallybloc s'exécute, puis l'exception est levée. Mais si une exception se produit finally, vous ne verrez jamais l'ancienne exception.

Vous pouvez le voir avec cet exemple:

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

Peu importe si Dispose()ou DisposeAsync()est appelé à l'intérieur du finally. Le comportement est le même.

Ma première pensée est: ne jetez pas Dispose(). Mais après avoir examiné une partie du code de Microsoft, je pense que cela dépend.

Jetez un œil à leur mise en œuvre FileStream, par exemple. Les deux la Dispose()méthode synchrone , et DisposeAsync()peuvent réellement lever des exceptions. La synchronisation Dispose()ne fait ignorer quelques exceptions intentionnellement, mais pas tous.

Mais je pense qu'il est important de prendre en compte la nature de votre classe. Dans un FileStream, par exemple, Dispose()videra le tampon dans le système de fichiers. C'est une tâche très importante et vous devez savoir si cela a échoué . Vous ne pouvez pas simplement ignorer cela.

Cependant, dans d'autres types d'objets, lorsque vous appelez Dispose(), vous n'avez vraiment plus besoin de l'objet. Appeler Dispose()signifie simplement "cet objet est mort pour moi". Peut-être qu'il nettoie une partie de la mémoire allouée, mais l'échec n'affecte en rien le fonctionnement de votre application. Dans ce cas, vous pourriez décider d'ignorer l'exception à l'intérieur de votre Dispose().

Mais dans tous les cas, si vous voulez faire la distinction entre une exception à l'intérieur de usingou une exception qui vient de Dispose(), alors vous avez besoin d'un bloc try/ catchà l'intérieur et à l'extérieur de votre usingbloc:

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 vous ne pourriez tout simplement pas utiliser using. Écrivez vous-même un bloc try/ catch/ finally, où vous interceptez toute exception dans 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
la source
3
Btw, source.dot.net (.NET Core) / referencesource.microsoft.com (.NET Framework) est beaucoup plus facile à parcourir que GitHub
canton7
Merci pour votre réponse! Je sais quelle est la vraie raison (j'ai mentionné le cas try / finalement et synchrone dans la question). Maintenant à propos de votre proposition. L' catch intérieur du usingbloc n'aiderait pas, car la gestion des exceptions se fait généralement quelque part loin du usingbloc lui-même, donc sa manipulation à l'intérieur usingn'est généralement pas très pratique. À propos de l'utilisation de non using- est-ce vraiment mieux que la solution de contournement proposée?
Vlad
2
@ canton7 Génial! J'étais au courant de referencesource.microsoft.com , mais je ne savais pas qu'il y avait un équivalent pour .NET Core. Merci!
Gabriel Luci
@Vlad "Better" est quelque chose auquel vous seul pouvez répondre. Je sais que si je lisais le code de quelqu'un d'autre, je préférerais voir try/ catch/ finallyblock car il serait immédiatement clair de ce qu'il fait sans avoir à lire ce qu'il AsyncUsingfait. Vous gardez également la possibilité de faire un retour anticipé. Il y aura également un coût supplémentaire pour votre CPU AwaitUsing. Ce serait petit, mais il est là.
Gabriel Luci
2
@PauloMorgado Cela signifie simplement qu'il Dispose()ne faut pas lancer car il est appelé plus d'une fois. Les propres implémentations de Microsoft peuvent lever des exceptions, et pour une bonne raison, comme je l'ai montré dans cette réponse. Cependant, je suis d'accord que vous devriez l'éviter si possible car personne ne s'attendrait normalement à ce qu'il lance.
Gabriel Luci
4

l'utilisation est en fait un code de gestion des exceptions (sucre de syntaxe pour essayer ... enfin ... Dispose ()).

Si votre code de gestion des exceptions lève des exceptions, quelque chose est royalement explosé.

Tout ce qui s'est passé pour vous y amener, n'a plus vraiment d'importance. Un code de gestion des exceptions défectueux masquera toutes les exceptions possibles, dans un sens ou dans l'autre. Le code de gestion des exceptions doit être fixe, avec une priorité absolue. Sans cela, vous n'obtenez jamais suffisamment de données de débogage pour le vrai problème. Je le vois très souvent mal. Il est aussi facile de se tromper que de manipuler des pointeurs nus. Si souvent, il y a deux articles sur le lien thématique I, qui pourraient vous aider avec toute conception erronée sous-jacente:

En fonction de la classification des exceptions, voici ce que vous devez faire si votre code de gestion des exceptions / dipose lève une exception:

Pour Fatal, Boneheaded et Vexing, la solution est la même.

Les exceptions exogènes doivent être évitées même à un coût élevé. Il y a une raison pour laquelle nous utilisons toujours des fichiers journaux plutôt que des bases de données pour consigner les exceptions - les opérations DB sont juste un moyen de s'exposer à des problèmes exogènes. Les fichiers journaux sont le seul cas, où cela ne me dérange même pas si vous gardez la poignée de fichier ouverte pendant toute la durée d'exécution.

Si vous devez fermer une connexion, ne vous inquiétez pas trop de l'autre extrémité. Traitez-le comme le fait UDP: "J'enverrai les informations, mais je m'en fiche si l'autre côté les obtienne." L'élimination consiste à nettoyer les ressources côté client / côté sur lequel vous travaillez.

Je peux essayer de les informer. Mais nettoyer les choses côté serveur / FS? C'est de cela que leurs délais d'attente et leur gestion des exceptions sont responsables.

Christophe
la source
Votre proposition se résume donc à supprimer les exceptions à la fermeture de la connexion, non?
Vlad
@Vlad Exogènes? Sûr. Dipose / Finalizer sont là pour nettoyer de leur côté. Il y a des chances que lors de la fermeture de l'instance Conneciton en raison d'une exception, vous le faites car vous n'avez plus de connexion fonctionnelle avec eux. Et quel intérêt y aurait-il à obtenir une exception "Aucune connexion" lors du traitement de l'exception "Aucune connexion" précédente? Vous envoyez un seul "Yo, I a close this connection" où vous ignorez toutes les exceptions exogènes ou même s'il se rapproche de la cible. Afaik les implémentations par défaut de Dispose le font déjà.
Christopher
@ Vlad: Je me suis souvenu qu'il y a un tas de choses dont vous n'êtes jamais censé lever des exceptions (à l'exception de celles qui sont fatales). Les initiateurs de type sont bien en haut de la liste. Dispose en fait également partie: «Pour garantir que les ressources sont toujours correctement nettoyées, une méthode Dispose doit pouvoir être appelée plusieurs fois sans lever d'exception. docs.microsoft.com/en-us/dotnet/standard/garbage-collection/…
Christopher
@Vlad La chance d'exceptions fatales? Nous devons toujours risquer ces risques et ne devons jamais les gérer au-delà de "appeler Dispose". Et ne devrait pas vraiment faire quoi que ce soit avec ceux-ci. Ils vont en fait sans mention dans aucune documentation. | Exceptions de tête en os? Réparez-les toujours. | Les exceptions de vexation sont des candidats privilégiés pour la déglutition / manipulation, comme dans TryParse () | Exogène? Aussi devrait toujours être manipulé. Souvent, vous souhaitez également en informer l'utilisateur et les enregistrer. Mais sinon, cela ne vaut pas la peine de tuer votre processus.
Christopher
@Vlad J'ai recherché SqlConnection.Dispose (). Il ne se soucie même pas d'envoyer quoi que ce soit au serveur concernant la connexion. Quelque chose pourrait encore se produire à la suite du NativeMethods.UnmapViewOfFile();et NativeMethods.CloseHandle(). Mais ceux-ci sont importés depuis extern. Il n'y a aucune vérification de toute valeur de retour ou de toute autre chose qui pourrait être utilisée pour obtenir une exception .NET appropriée autour de ce que ces deux pourraient rencontrer. Je suppose donc fortement que SqlConnection.Dispose (bool) s'en fiche tout simplement. | Fermer est beaucoup plus agréable à dire au serveur. Avant d'appeler disposer.
Christopher
1

Vous pouvez essayer d'utiliser AggregateException et modifier votre code comme ceci:

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
la source