Disposing a static brush
-
19-08-2019 - |
Question
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 ?
Solution
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.
OTHER TIPS
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.
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.