Une fuite de mémoire est-elle créée si un MemoryStream dans .NET n'est pas fermé?

112

J'ai le code suivant:

MemoryStream foo(){
    MemoryStream ms = new MemoryStream();
    // write stuff to ms
    return ms;
}

void bar(){
    MemoryStream ms2 = foo();
    // do stuff with ms2
    return;
}

Y a-t-il une chance que le MemoryStream que j'ai alloué échoue d'une manière ou d'une autre à être détruit plus tard?

J'ai un examen par les pairs qui insiste pour que je ferme manuellement cela, et je ne trouve pas les informations pour dire s'il a un point valide ou non.

Codeur
la source
41
Demandez à votre critique exactement pourquoi il pense que vous devriez le fermer. S'il parle de bonnes pratiques générales, il est probablement intelligent. S'il parle de libérer la mémoire plus tôt, il a tort.
Jon Skeet

Réponses:

60

Si quelque chose est jetable, vous devez toujours le jeter. Vous devriez utiliser une usingdéclaration dans votre bar()méthode pour vous assurer qu'elle ms2est éliminée.

Il sera éventuellement nettoyé par le garbage collector, mais il est toujours recommandé d'appeler Dispose. Si vous exécutez FxCop sur votre code, il le signalera comme un avertissement.

Rob Prouse
la source
16
Les appels de bloc utilisant se débarrassent pour vous.
Nick
20
@Grauenwolf: votre assertion rompt l'encapsulation. En tant que consommateur, vous ne devriez pas vous soucier de savoir si c'est no-op: s'il est IDisposable, c'est votre travail de le Dispose ().
Marc Gravell
4
Ce n'est pas vrai pour la classe StreamWriter: il supprimera le flux connecté uniquement si vous supprimez le StreamWriter - il ne supprimera jamais le flux s'il est récupéré et que son finaliseur est appelé - c'est par conception.
springy76
4
Je sais que cette question remonte à 2008, mais aujourd'hui, nous avons la bibliothèque de tâches .NET 4.0. Dispose () n'est pas nécessaire dans la plupart des cas lors de l'utilisation de Tasks. Bien que je convienne que IDisposable devrait signifier "Vous feriez mieux de vous débarrasser de cela lorsque vous avez terminé", cela ne signifie plus vraiment cela.
Phil
7
Un autre exemple que vous ne devez pas supprimer d'objet IDisposable est HttpClient aspnetmonsters.com/2016/08/2016-08-27-httpclientwrong Juste un autre exemple de BCL où il y a un objet IDisposable et vous n'avez pas besoin (ou même ne devriez pas) de disposer il. C'est juste pour se rappeler qu'il y a généralement des exceptions à la règle générale, même en BCL;)
Mariusz Pawelski
166

Vous ne fuirez rien - du moins dans l'implémentation actuelle.

L'appel de Dispose ne nettoiera pas plus rapidement la mémoire utilisée par MemoryStream. Il va arrêter votre flux d'être viable en lecture / écriture des appels après l'appel, qui peut ou peut ne pas être utile pour vous.

Si vous êtes absolument sûr de ne jamais vouloir passer d'un MemoryStream à un autre type de flux, cela ne vous fera aucun mal de ne pas appeler Dispose. Cependant, c'est généralement une bonne pratique en partie parce que si jamais vous changez pour utiliser un autre Stream, vous ne voulez pas être mordu par un bogue difficile à trouver parce que vous avez choisi la solution de facilité dès le début. (D'un autre côté, il y a l'argument YAGNI ...)

L'autre raison de le faire de toute façon est qu'une nouvelle implémentation peut introduire des ressources qui seraient libérées sur Dispose.

Jon Skeet
la source
Dans ce cas, la fonction renvoie un MemoryStream car il fournit «des données qui peuvent être interprétées différemment selon les paramètres d'appel», donc cela aurait pu être un tableau d'octets, mais c'était plus facile pour d'autres raisons de faire comme MemoryStream. Ce ne sera donc certainement pas une autre classe Stream.
Coderer
Dans ce cas, j'essaierais toujours de m'en débarrasser uniquement sur un principe général - adopter de bonnes habitudes, etc. - mais je ne m'inquiéterais pas trop si cela devenait délicat.
Jon Skeet
1
Si vous craignez vraiment de libérer des ressources dès que possible, annulez la référence immédiatement après votre bloc "using", de sorte que les ressources non gérées (s'il y en a) sont nettoyées et l'objet devient éligible pour le garbage collection. Si la méthode revient tout de suite, cela ne fera probablement pas beaucoup de différence, mais si vous continuez à faire d'autres choses dans la méthode comme demander plus de mémoire, cela peut certainement faire une différence.
Triynko
@Triynko Pas vraiment vrai: voir: stackoverflow.com/questions/574019/… pour plus de détails.
George Stocker
10
L'argument YAGNI pourrait être pris dans les deux sens - puisque décider de ne pas disposer de quelque chose qui implémente IDisposableest un cas particulier allant à l'encontre des meilleures pratiques normales, vous pouvez faire valoir que c'est ce cas que vous ne devriez pas faire avant d'en avoir vraiment besoin, dans le cadre du YAGNI principe.
Jon Hanna
26

Oui, il y a une fuite , en fonction de la façon dont vous définissez FUITE et de combien plus tard vous voulez dire ...

Si par fuite vous entendez "la mémoire reste allouée, indisponible pour une utilisation, même si vous avez fini de l'utiliser" et que vous entendez par la dernière fois à tout moment après avoir appelé dispose, alors oui il peut y avoir une fuite, bien que ce ne soit pas permanent (c'est-à-dire pour la durée de vie de vos applications).

Pour libérer la mémoire managée utilisée par MemoryStream, vous devez l' annuler en annulant votre référence à celle-ci, afin qu'elle devienne immédiatement éligible pour le garbage collection. Si vous ne le faites pas, vous créez une fuite temporaire à partir du moment où vous avez fini de l'utiliser, jusqu'à ce que votre référence soit hors de portée, car entre-temps la mémoire ne sera pas disponible pour l'allocation.

L'avantage de l'instruction using (par rapport à un simple appel à disposer) est que vous pouvez DÉCLARER votre référence dans l'instruction using. Lorsque l'instruction using se termine, non seulement Dispose est appelé, mais votre référence sort de la portée, annulant effectivement la référence et rendant immédiatement votre objet éligible au garbage collection sans que vous ayez à vous rappeler d'écrire le code "reference = null".

Si le fait de ne pas déférer tout de suite quelque chose n'est pas une fuite de mémoire "permanente" classique, cela a certainement le même effet. Par exemple, si vous conservez votre référence au MemoryStream (même après avoir appelé dispose), et un peu plus loin dans votre méthode, vous essayez d'allouer plus de mémoire ... la mémoire utilisée par votre flux mémoire encore référencé ne sera pas disponible jusqu'à ce que vous annuliez la référence ou qu'elle soit hors de portée, même si vous avez appelé dispose et que vous avez fini de l'utiliser.

Triynko
la source
6
J'adore cette réponse. Parfois, les gens oublient le double devoir d'utiliser: la récupération avide des ressources et le déréférencement avide.
Kit
1
En effet, bien que j'aie entendu dire que contrairement à Java, le compilateur C # détecte la "dernière utilisation possible", donc si la variable est destinée à sortir du champ d'application après sa dernière référence, elle peut devenir éligible au garbage collection juste après sa dernière utilisation possible. avant qu'il ne sorte réellement du champ d'application. Voir stackoverflow.com/questions/680550/explicit-nulling
Triynko
2
Le ramasse-miettes et la gigue ne fonctionnent pas de cette façon. Scope est une construction de langage et non quelque chose auquel le runtime obéira. En fait, vous augmentez probablement la durée de conservation de la référence en ajoutant un appel à .Dispose () lorsque le bloc se termine. Voir ericlippert.com/2015/05/18
Pablo Montilla
8

L'appel .Dispose()(ou l'encapsulation avec Using) n'est pas requis.

La raison pour laquelle vous appelez .Dispose()est de libérer la ressource dès que possible .

Pensez en termes, par exemple, au serveur Stack Overflow, où nous avons un ensemble limité de mémoire et des milliers de requêtes entrantes. Nous ne voulons pas attendre la récupération de place planifiée, nous voulons libérer cette mémoire dès que possible afin qu'elle soit disponible pour les nouvelles demandes entrantes.

Jeff Atwood
la source
24
L'appel de Dispose sur un MemoryStream ne libère cependant aucune mémoire. En fait, vous pouvez toujours obtenir les données dans un MemoryStream après avoir appelé Dispose - essayez-le :)
Jon Skeet
12
-1 Bien que ce soit vrai pour un MemoryStream, en tant que conseil général, c'est tout simplement faux. Dispose consiste à libérer les ressources non gérées , telles que les descripteurs de fichiers ou les connexions à la base de données. La mémoire n'entre pas dans cette catégorie. Vous devez presque toujours attendre le garbage collection planifié pour libérer de la mémoire.
Joe
1
Quel est l'avantage d'adopter un style de codage pour l'allocation et la suppression des FileStreamobjets et un autre pour les MemoryStreamobjets?
Robert Rossney
3
Un FileStream implique des ressources non gérées qui pourraient en fait être immédiatement libérées lors de l'appel de Dispose. Un MemoryStream, en revanche, stocke un tableau d'octets géré dans sa variable _buffer, qui n'est pas libérée au moment de l'élimination. En fait, le _buffer n'est même pas annulé dans la méthode Dispose de MemoryStream, qui est un SHAMEFUL BUG IMO car l'annulation de la référence pourrait rendre la mémoire éligible pour GC juste au moment de l'élimination. Au lieu de cela, une référence MemoryStream persistante (mais supprimée) conserve toujours la mémoire. Par conséquent, une fois que vous l'avez supprimé, vous devez également l'annuler s'il est toujours dans la portée.
Triynko
@Triynko - "Par conséquent, une fois que vous l'avez éliminé, vous devriez également l'annuler s'il est toujours dans la portée" - Je ne suis pas d'accord. S'il est à nouveau utilisé après l'appel à Dispose, cela provoquera une NullReferenceException. S'il n'est pas réutilisé après Dispose, il n'est pas nécessaire de l'annuler; le GC est assez intelligent.
Joe
8

Cela a déjà été répondu, mais j'ajouterai simplement que le bon vieux principe de masquage d'informations signifie que vous voudrez peut-être à un moment donné refactoriser:

MemoryStream foo()
{    
    MemoryStream ms = new MemoryStream();    
    // write stuff to ms    
    return ms;
}

à:

Stream foo()
{    
   ...
}

Cela met l'accent sur le fait que les appelants ne doivent pas se soucier du type de Stream renvoyé, et permet de changer l'implémentation interne (par exemple lors de la simulation pour les tests unitaires).

Vous devrez alors avoir des problèmes si vous n'avez pas utilisé Dispose dans votre implémentation de barre:

void bar()
{    
    using (Stream s = foo())
    {
        // do stuff with s
        return;
    }
}
Joe
la source
5

Tous les flux implémentent IDisposable. Enveloppez votre flux de mémoire dans une déclaration d'utilisation et tout ira bien. Le bloc using garantira que votre flux est fermé et supprimé quoi qu'il arrive.

partout où vous appelez Foo, vous pouvez le faire en utilisant (MemoryStream ms = foo ()) et je pense que vous devriez toujours être ok.

pseudo
la source
1
Un problème que j'ai rencontré avec cette habitude est que vous devez être sûr que le flux n'est utilisé nulle part ailleurs. Par exemple, j'ai créé un JpegBitmapDecoder pointé vers un MemoryStream et renvoyé Frames [0] (en pensant qu'il copierait les données dans son propre magasin interne) mais j'ai trouvé que le bitmap ne s'afficherait que 20% du temps - il s'est avéré que c'était parce que Je supprimais le flux de mémoire.
devios1
Si votre flux mémoire doit persister (c'est-à-dire qu'un bloc using n'a pas de sens), alors vous devez appeler Dispose et définir immédiatement la variable sur null. Si vous le supprimez, il n'est plus destiné à être utilisé, vous devez donc le définir immédiatement sur null. Ce que chaiguy décrit ressemble à un problème de gestion des ressources, car vous ne devriez pas donner de référence à quelque chose à moins que la chose à laquelle vous le donnez prenne la responsabilité de l'éliminer, et la chose qui distribue la référence sait qu'elle n'est plus responsable de Ce faisant.
Triynko
2

Vous ne fuirez pas de mémoire, mais votre réviseur de code a raison d'indiquer que vous devez fermer votre flux. C'est poli de le faire.

La seule situation dans laquelle vous pourriez perdre de la mémoire est lorsque vous laissez accidentellement une référence au flux et ne le fermez jamais. Vous n'êtes toujours pas une fuite de mémoire vraiment, mais vous êtes étendez inutilement la quantité de temps que vous prétendez l'utiliser.

OwenP
la source
1
> Vous ne perdez toujours pas vraiment de mémoire, mais vous prolongez inutilement la durée pendant laquelle vous prétendez l'utiliser. Êtes-vous sûr? Dispose ne libère pas de mémoire et son appel tard dans la fonction peut en fait prolonger la durée pendant laquelle il ne peut pas être collecté.
Jonathan Allen
2
Ouais, Jonathan a raison. Placer un appel à Dispose tard dans la fonction pourrait en fait amener le compilateur à penser que vous devez accéder à l'instance de flux (pour la fermer) très tard dans la fonction. Cela pourrait être pire que de ne pas appeler du tout dispose (évitant ainsi une référence tardive à la variable de flux), car un compilateur pourrait sinon calculer un point de publication optimal (alias "point de dernière utilisation possible") plus tôt dans la fonction .
Triynko
2

Je recommanderais d'encapsuler le MemoryStream bar()dans une usinginstruction principalement pour la cohérence:

  • À l'heure actuelle, MemoryStream ne libère pas de mémoire .Dispose(), mais il est possible qu'à un moment donné dans le futur, il le soit, ou vous (ou quelqu'un d'autre dans votre entreprise) puissiez le remplacer par votre propre MemoryStream personnalisé, etc.
  • Cela aide à établir un modèle dans votre projet pour garantir que tous les flux sont supprimés - la ligne est plus fermement tracée en disant "tous les flux doivent être supprimés" au lieu de "certains flux doivent être supprimés, mais certains ne le sont pas" ...
  • Si jamais vous modifiez le code pour permettre le renvoi d'autres types de flux, vous devrez le changer pour le supprimer de toute façon.

Une autre chose que je fais habituellement dans des cas comme foo()lors de la création et du retour d'un IDisposable est de m'assurer que tout échec entre la construction de l'objet et le returnest intercepté par une exception, supprime l'objet et renvoie l'exception:

MemoryStream x = new MemoryStream();
try
{
    // ... other code goes here ...
    return x;
}
catch
{
    // "other code" failed, dispose the stream before throwing out the Exception
    x.Dispose();
    throw;
}
Chris R. Donnelly
la source
1

Si un objet implémente IDisposable, vous devez appeler la méthode .Dispose lorsque vous avez terminé.

Dans certains objets, Dispose signifie la même chose que Fermer et vice versa, dans ce cas, l'un ou l'autre est bon.

Maintenant, pour votre question particulière, non, vous ne fuirez pas de mémoire.

Lasse V. Karlsen
la source
3
«Must» est un mot très fort. Chaque fois qu'il y a des règles, il vaut la peine de connaître les conséquences de leur violation. Pour MemoryStream, il y a très peu de conséquences.
Jon Skeet
-1

Je ne suis pas un expert .net, mais peut-être que le problème ici est les ressources, à savoir le descripteur de fichier, et non la mémoire. Je suppose que le ramasse-miettes finira par libérer le flux et fermer la poignée, mais je pense qu'il serait toujours préférable de le fermer explicitement, pour vous assurer de vider le contenu sur le disque.

Steve
la source
Un MemoryStream est entièrement en mémoire - il n'y a pas de descripteur de fichier ici.
Jon Skeet
-2

L'élimination des ressources non gérées n'est pas déterministe dans les langages de récupération de place. Même si vous appelez Dispose explicitement, vous n'avez absolument aucun contrôle sur le moment où la mémoire de sauvegarde est réellement libérée. Dispose est implicitement appelé lorsqu'un objet sort de la portée, que ce soit en quittant une instruction using ou en faisant apparaître la pile d'appels à partir d'une méthode subordonnée. Tout cela étant dit, parfois l'objet peut en fait être un wrapper pour une ressource gérée (par exemple un fichier). C'est pourquoi il est recommandé de fermer explicitement les instructions finally ou d'utiliser l'instruction using. À votre santé


la source
1
Pas exactement vrai. Dispose est appelé lors de la sortie d'une instruction using. Dispose n'est pas appelé lorsqu'un objet sort juste de la portée.
Alexander Abramov
-3

MemorySteram n'est rien d'autre qu'un tableau d'octets, qui est un objet géré. Oubliez de jeter ou de fermer cela n'a aucun effet secondaire autre que sur la tête de la finalisation.
Vérifiez simplement le constuctor ou la méthode de rinçage de MemoryStream dans le réflecteur et vous comprendrez pourquoi vous n'avez pas à vous soucier de sa fermeture ou de sa mise au rebut autrement que pour suivre les bonnes pratiques.

user1957438
la source
6
-1: Si vous souhaitez publier une question datant de 4 ans et plus avec une réponse acceptée, essayez d'en faire quelque chose d'utile.
Tieson T.