Discussion :

[Axiome]

Bonjour,

Avec ce code :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
 
private void button1_Click(object sender, EventArgs e)
        {
            PictureBox pb = new PictureBox();
            pb.Image = new Bitmap("c:\\image.jpg");
            this.Controls.Add(pb);
        }

on remarque bien qu'il y a une fuite mémoire, à chaque fois qu'on clique sur le bouton on charge la bitmap sans jamais la libérer.

Pour remédier à ce problème, j'ai opté pour cette solution :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
 
private PictureBox pb;
private void button1_Click(object sender, EventArgs e)
        {
            if (pb != null)
                pb.Dispose();
 
            pb = new PictureBox();
            pb.Image = new Bitmap("c:\\image.jpg");
            this.Controls.Add(pb);
            GC.Collect();
        }

Qu'en pensez-vous de cette solution ? existe-t-il une autre solution plus élégante ?

Merci
+

[shwin]

Il y as pas de fuite de mémoire, ta picturebox va se faire disposé par le GarbageCollector car il y a plus de référence dessus. Donc quand le GC va faire son Collect(), ca va etre réglé.

Il y as pas de memoryleak en c# justement grace au GC

[Axiome]

Je ne suis pas tout à fait d'accord. Il suffit de tester l'exemple de préférence avec une image de grande taille. Si on clique une dizaine de fois sur le bouton, on risque de charger la mémoire pour rien avec mon premier code. Même en rajoutant un GC.Collect l'espace mémoire occupé par l'application ne cesse d'augmenter.
Alors qu'avec mon deuxième code, l'espace mémoire est tjs stable une fois que l'image est chargée la première fois. Je pense que c'est une optimisation non négligeable au niveau de la mémoire. Mais ce qui me gène c'est le fait que ma variable qui était locale passe en tant que membre privé dans la classe alors qu'il n'y a pas un réel besoin vu qu'elle n'est utilisée que dans ce bloc de code.

Vos avis m'intéresse.

NB : Vous pouvez tester cet exemple pour ceux qui me diront que le garbage collector s'occupe de tout. Ce qu'il faut voir c'est cette effet sur une application de grande envergure où on risque de faire saturer la mémoire très rapidement avec une telle utilisation (Code 1).

[shwin]

Ta perception sur le memory manager est tout simplement fausse. C'est beaucoup plus compliquer que ce que tu pense.

Si windows manque de mémoire, il va dire au GC de faire un Collect pour ramassé les objects sans références. Mais tant que windows à pas besoin de cette espace, la mémoire reste alloué jusqua ce que le GC décide de faire son Collect().

Bref, ca donne rien de faire un Collect() si tu utilise 100meg/1000.

Je sais pas trop si tu comprend le principe.

Dans ton exemple, laisse le GC faire sa job. Il est beaucoup mieux codé que le code que toi et moi codons

Faire un dispose sur un object est utile si son lifetime est tres court, genre un sqlcommand, ou un stream.


using(SqlCommand = new SqlCommand())
{
...
}

Ca c'est utile pour le dispose mais dans ton exemple, le GC est assé grand pour faire sa job!

[Graffito]

L'instruction

if (pb != null) pb.Dispose();

ne suffit pas puisque pb a été ajouté au controls de la form et il reste donc une référence au picturebox pb malgré le Dispose, d'où l'inefficacité du GC.

Il faut aussi "soustraire" pb des controls :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

this.Controls.Remove(pb) ;

[Guulh]

En plus, rien ne sert de supprimer le picture box pour le rajouter juste apres... Sans jamais le retirer des controles, il sufit de modifier sa propriete Image. Et il doit valoir mieux Disposer cette image avant d'en changer, histoire de liberer le fichier, meme si du coup comme t'as plus ed reference dessus, le GC finira par le nettoyer.

[Immobilis]

Salut,

Pour remédier à ce problème, j'ai opté pour cette solution :

(...)

Qu'en pensez-vous de cette solution ? existe-t-il une autre solution plus élégante ?

Il faut laisser le GC collector faire son taf. Si un objet est disposable, il faut l'utiliser dans un "using"

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
 
using (PictureBox pb = new PictureBox())
{
    pb.Image = new Bitmap("c:\\image.jpg");
    this.Controls.Add(pb);
}

Un objet disposable dans un using ne vit pas plus longtemps que l'accolade de fermeture.
Plus généralement, une classe qui utilise un objet disposable en dehors d'un using doit être elle même Disposable (implémenter IDisposable). Les objets disposables déclarés dans cette classe doivent être disposés dans la méthode Dispose.
Si tu créés une classe A utilisant un objet PictureBox, A doit implémenter IDisposable.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
 
public class A: IDisposable
{
       private PictureBox pb = new PictureBox();
 
       public void Dispose()
       {
             pb.Dispose();
       }
}

Il y a un pattern particulier pour cela.
Pour utiliser A tu fais:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
 
using(A a = new A())
{
     // Le reste de ton code
}

Je te conseille d'utiliser FXCop pour tout tes dev.

A+

[Immobilis]

Pour être très précis et élégant comme tu dis (je laisse les commentaires de l'auteur M$?): http://msdn.microsoft.com/en-us/libr...le(VS.80).aspx. Je ne suis plus certain d'avoir trouvé ce code à cet endroit mais bon...

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
 
public sealed class A : IDisposable
{
    private PictureBox pb = new PictureBox();
    // Variable to track if Dispose has been called
    private bool disposed = false;
 
    public A() { }
 
    #region IDisposable
 
    /// <summary>
    /// <c>A</c> creates members of the following IDisposable types: 
    /// <c>AProvider</c>. If <c>A</c> has previously shipped,
    /// adding new members that implement IDisposable to this 
    /// type is considered a breaking change to existing consumers.
    /// </summary>
    public void Dispose()
    {
        // Check if Dispose has already been called 
        if (!disposed)
        {
            // Call the overridden Dispose method that contains common cleanup code
            // Pass true to indicate that it is called from Dispose
            Dispose(true);
            // Prevent subsequent finalization of this object. This is not needed 
            // because managed and unmanaged resources have been explicitly released
            GC.SuppressFinalize(this);
        }
    }
 
    /// <summary>
    /// Implement a finalizer by using destructor style syntax.
    /// Use of this destructor will be done by GC.
    /// </summary>
    ~A()
    {
        // Call the overridden Dispose method that contains common cleanup code
        // Pass false to indicate the it is not called from Dispose
        Dispose(false);
    }
 
    /// <summary>
    /// Implement the override Dispose method that will contain common cleanup functionality
    /// </summary>
    /// <param name="disposing">This parameter indicates whether disposition of object is managed by user's coding or GC cleanup.</param>
    private void Dispose(bool disposing)
    {
        if (disposing)
        {
            pb.Dispose();
        }
        disposed = true;
    }
 
    #endregion
}

J'utilise cette "disposition" pour tout mes objets qui utilisent des objets de connexion à ma base de données.

Sinon

Dans ton exemple, laisse le GC faire sa job. Il est beaucoup mieux codé que le code que toi et moi codons

Me fait dresser les cheveux. Evidement, le GC essaye de se dépatouiller avec le code de m**** que certains codeurs font en se disant: "pas de soucis, le GC est pour ramasser les objets qui trainent". C'est évidement très risqué parce que des fois ben le pauvre GC n'y arrive plus.

NB : Vous pouvez tester cet exemple pour ceux qui me diront que le garbage collector s'occupe de tout. Ce qu'il faut voir c'est cette effet sur une application de grande envergure où on risque de faire saturer la mémoire très rapidement avec une telle utilisation (Code 1).

Excellente démarche, mais il faut pousser plus loin. Est-ce que tu utilises FXCop, les compteurs de performances?

A+

[olibara]

Bonjour je pense que le code ci dessous réponds a la question initiale

L'idée principale est de passer par un stream que l'on fermera ou disposera explicitement !

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
      FileStream fs = null;
      try
      {
        fs = new FileStream(name, FileMode.Open, FileAccess.Read);
        img = Image.FromStream(fs,true,true);
      }
      finally
      {
        fs.Close();
      }

[Immobilis]

L'idée principale est de passer par un stream que l'on fermera ou disposera explicitement !

Raahh

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
 
try
{
    using (FileStream fs = new FileStream(@"C:\dossier\fichier.jpg", FileMode.Open, FileAccess.Read))
    {
         // Code
    }
}
catch (Exception)
{
    throw;
}

fs.Close ne suffit pas. Faites des using c'est tellement plus simple! Et comme ça on oublie rien, ni le close, ni le dispose.
Qu'est-ce que c'est "Image" dans "Image.FromStream"?

A+

[olibara]

D'accord d'accord !
Tu a raison

Du calme Respire !

Pour ta question

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

img = Image.FromStream(fs,true,true);

img c'est précisement l'image qu'il va utiliser sur son picturebox
Dans mon exemple le name c'est "c:\\image.jpg"

C'est pas bon de faire

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

pb.Image = new Bitmap("c:\\image.jpg");

En général ces type d'acces direct aux fichiers image laissent le fichier ouvert durant toute la session !

[Immobilis]

Du calme Respire !

Arf désolé, je m'emporte.

Je ne connais pas PictureBox. Mais qu'est-ce que "Image" dans

img = Image.FromStream(fs,true,true);

Merki

[olibara]

Salut

img = Image.FromStream(fs,true,true);

Ben c'est une classe Image qui a une methode FromStream qui permet de construire une image a partir d'un fichier jpg, bmp etc..

La classe Image est native au FW tout comme le PictureBox pour lequel Axiome veut charger un bmp

[Immobilis]

Quel namespace?

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

System.Web.UI.WebControls.Image

Je trouve pas la méthode.

A+

[olibara]

Salut

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

System.Drawing.Image

[Immobilis]

Arf, je suis pas coutumier des WinForms.

Merki

[Axiome]

Merci à tous pour vos réponses, je remarque que les réponses convergent vers l'utilisation d'un using sauf que cela n'a aucun intérêt pour mon code.
Il suffit de faire le test, d'ailleurs Immobilis donne bien l'intérêt des using

Un objet disposable dans un using ne vit pas plus longtemps que l'accolade de fermeture.

du coup l'image est rapidement disposé (juste après l'accolade de fermeture) et ne sera pas visible dans la forme après le clique sur le bouton.

Au fait, je ne cherche pas la façon de charger une image avec new bitmap ou stream, j'ai juste essayé d'illustrer mon problème avec un exemple simple et facile à tester.

Mes interrogations restent entière ???

[olibara]

Salut

1- Deja si tu déclarais le picture Box en membre privé tu ne devrais pas en instancier un nouveau a chaque fois que tu appuye sur le bouton

2- Ensuite si tu utilise la methode de lecture que je t"ai proposée via un stream que tu declare dans un using, tu ne fera que changer l'image de ton seul et unique PictureBox sans intancier chaque fois un nouveau bitmap

Mais en faisant comme tu fais c'est normal que le GC doive faire le menage pour toi !

[Immobilis]

et ne sera pas visible dans la forme après le clique sur le bouton.

Ah bon? IL faut peut-être effectivement déclarer le picturebox en membre privé et rendre ta classe disposable.

[Axiome]

1- Deja si tu déclarais le picture Box en membre privé tu ne devrais pas en instancier un nouveau a chaque fois que tu appuye sur le bouton

Tu as tout à fait raison, mais n'oublies pas qu'ici je donne un exemple simple en utilisant le framework .net et que mon problème est plus complexe que ça par ce que j'utilise un framework propriétaire et je suis obligé de faire une instanciation de mon objet lors du clique sur le bouton.