Disposing a static brush

Question

Hi, I'm writing a biorhythm app. To test it i have a form with a Button and a PictureBox. When I click on the button i do

myPictureBox.Image = GetBiorhythm2();

Which runs ok for the first time, but on the second click it causes the following exception:

System.ArgumentException: Parameter is not valid.
   at System.Drawing.Graphics.CheckErrorStatus
   at System.Drawing.Graphics.FillEllipse
   at Larifari.Biorhythm.Biorhythm.GetBiorhythm2 in c:\delo\Horoskop\Biorhythm.cs:line 157
   at Larifari.test.Button1Click in c:\delo\Horoskop\test.Designer.cs:line 169
   at System.Windows.Forms.Control.OnClick
   at System.Windows.Forms.Button.OnClick
   at System.Windows.Forms.Button.OnMouseUp
   at System.Windows.Forms.Control.WmMouseUp
   at System.Windows.Forms.Control.WndProc
   at System.Windows.Forms.ButtonBase.WndProc
   at System.Windows.Forms.Button.WndProc
   at ControlNativeWindow.OnMessage
   at ControlNativeWindow.WndProc
   at System.Windows.Forms.NativeWindow.DebuggableCallback
   at ComponentManager.System.Windows.Forms.UnsafeNativeMethods.IMsoComponentManager.FPushMessageLoop
   at ThreadContext.RunMessageLoopInner
   at ThreadContext.RunMessageLoop
   at System.Windows.Forms.Application.Run
   at Larifari.test.Main in c:\delo\Horoskop\test.cs:line 20

the cut-down function which causes the error is :

public static Image GetBiorhythm2() {
        Bitmap bmp = new Bitmap(600, 300);
        Image img = bmp;
        Graphics g = Graphics.FromImage(img);

        Brush brush = Brushes.Black;
        g.FillEllipse(brush, 3, 3, 2, 2); //Here the exception is thrown on the second call to the function

        brush.Dispose(); //If i comment this out, it works ok.

        return img;
 }

if I comment-out the brush disposal it works ok, but I am not happy with that and wish to find an alternative solution. Can you help me please ?

Answer 1

I don't think you need to call .Dispose on static brushes, only if you create new ones. Although, personally, I would use the using syntax.. ie:

using (Brush brush = new SolidBrush(...))
{
    g.FillEllipse(brush, 3, 3, 2, 2);
}

And you should probably do the same thing with the graphics object you create.

Answer 2

Bruhes.Black is a system resource, and is not intended for you to dispose. The runtime manages the brushes in the Brushes class, the Pens, and other such objects. It creates and disposes those objects as required, keeping frequently-used items alive so that it doesn't have to continually create and destroy them.

The documentation for the Brushes class says:

The Brushes class contains static read-only properties that return a Brush object of the color indicated by the property name. You typically do not have to explicitly dispose of the brush returned by a property in this class, unless it is used to construct a new brush.

In short, do not call Dispose on the system-supplied objects.

Answer 3

It looks like you're trying to dispose of a static, which causes some problems next time it's used:

    Brush brush = Brushes.Black;
    g.FillEllipse(brush, 3, 3, 2, 2); //Here the exception is thrown on the second call to the function

    brush.Dispose(); //If i comment this out, it works ok.

When you set brush = Brushes.Black, you're actually setting brush as a reference (or pointer) to the static Brushes.Black. By disposing it, you're effectively writing:

    Brushes.Black.dispose();

When you come back around to use the black brush again, the runtime says you can't because it's already been disposed of, and isn't a valid argument to g.FillEllipse()

A better way to write this might be just simply:

    g.FillEllipse(Brushes.Black, 3, 3, 2, 2);

Or, if you want to be really complex about it:

    Brush brush = Brushes.Black.Clone();
    g.FillEllipse( brush, 3, 3, 2, 2 );
    brush.Dispose();

Or if you don't care about things looking wrong, just comment out the brush.Dispose(); line in your original code.