c# Pen throws OutOfMemoryException

Question

A form with a panel and boxes drawn on, the user clicks a box and a label appears for a moment to say "box hit".

I have a working version of the code but I tried to rearrange it to experiment with making it more efficient, and I find I am currently getting this exception.

I do not understand why this exception is occurring, and I would like to be able to troubleshoot it but I don't know how to do that either. I have narrowed it down to what line it is. But beyond that I don't know.

The error occurs when a box is clicked.. So the panel's mousedown is executed.

All the controls e.g. panel, label, are programmatically generated, such that it should hopefully be possible for anybody to copy/paste the code change the classname and reproduce the exception that I get.

I have read online that the pen might generate an outofmemoryexception even when it's not really out of memory, but it's just throwing that exception, though that still doesn't tell me why or how it can be avoided, so that I can understand how to write code where the pen doesn't throw that exception.

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;
using System.Drawing.Drawing2D;

namespace WindowsFormsApplication17
{
    public partial class Form1 : Form
    {
        Label lbl1;
        Panel panel;
        Timer timer1;

        Point[][] points = new Point[9][];

        GraphicsPath[] gps = new GraphicsPath[9];

        public Form1()
        {
            InitializeComponent();
        }

        private void Form1_Load(object sender, EventArgs e)
        {
            panel = new Panel();


            lbl1 = new Label();
            timer1 = new Timer();

            this.Controls.Add(panel);

            Label alias = lbl1;
            alias.Text = "box clicked";
            alias.Visible = false;

            panel.BackgroundImageLayout = ImageLayout.Stretch;
            this.Height = 500; this.Width = 500;


            panel.Controls.Add(lbl1);

            panel.MouseDown += p_MouseDown;
            timer1.Tick += timer1_Tick;

            panel.BackColor = Color.White;
            panel.Height = 400;
            panel.Width = 400;


            for (int i = 0; i < 9; i++) points[i] = new Point[4];

            points[0][0] = new Point(19, 262);
            points[0][1] = new Point(28, 257);
            points[0][2] = new Point(27, 284);
            points[0][3] = new Point(16, 285);


            points[1][0] = new Point(52, 253);
            points[1][1] = new Point(62, 250);
            points[1][2] = new Point(61, 277);
            points[1][3] = new Point(49, 278);


            points[2][0] = new Point(87, 249);
            points[2][1] = new Point(100, 248);
            points[2][2] = new Point(99, 275);
            points[2][3] = new Point(86, 274);

            points[3][0] = new Point(126, 250);
            points[3][1] = new Point(140, 252);
            points[3][2] = new Point(139, 279);
            points[3][3] = new Point(126, 277);


            points[4][0] = new Point(164, 257);
            points[4][1] = new Point(175, 260);
            points[4][2] = new Point(175, 287);
            points[4][3] = new Point(164, 284);

            points[5][0] = new Point(197, 265);
            points[5][1] = new Point(209, 269);
            points[5][2] = new Point(209, 295);
            points[5][3] = new Point(198, 292);


            points[6][0] = new Point(228, 273);
            points[6][1] = new Point(241, 275);
            points[6][2] = new Point(240, 300);
            points[6][3] = new Point(229, 300);

            points[7][0] = new Point(262, 274);
            points[7][1] = new Point(274, 273);
            points[7][2] = new Point(275, 300);
            points[7][3] = new Point(262, 301);

            points[8][0] = new Point(297, 272);
            points[8][1] = new Point(308, 268);
            points[8][2] = new Point(311, 295);
            points[8][3] = new Point(298, 296);


            panel.Paint += thepanel_Paint;


            // I can see that I can remove all the braces, but anyhow.
            for (int i = 0; i < 9; i++)
            {
                using (gps[i] = new GraphicsPath())
                using (Pen pen = new Pen(Color.Black, 1))  //this pen is not for drawing. but for the logic re graphicspath
                {
                    gps[i].AddPolygon(points[i]);                  
                }
            }


        }




        void p_MouseDown(object sender, EventArgs e)
        {

            MouseEventArgs mea = (MouseEventArgs)e;

            //GraphicsPath[] gps = new GraphicsPath[9];
            bool boxhit = false;


            for (int i = 0; i < 9; i++)
            {
                using (Pen pen = new Pen(Color.Black, 1))  //this pen is not for drawing. but for the logic re graphicspath
                {

                    bool cond1=false; 
                    bool cond2=false;  


                    cond1 = gps[i].IsOutlineVisible(new Point(mea.X, mea.Y), pen);
                    cond2 = gps[i].IsVisible(new Point(mea.X, mea.Y));



                    if (cond1 == true ||
                        cond2 == true)
                    {
                        boxhit = true;
                        lbl1.Visible = true;
                        timer1.Enabled = true;
                    }


                }


            } //for

            // if (boxhit == false) MessageBox.Show("no box hit");


        }


        private void timer1_Tick(object sender, EventArgs e)
        {
            lbl1.Visible = false; timer1.Enabled = false;

        }



        private void thepanel_Paint(object sender, PaintEventArgs e)
        {
            GraphicsPath[] gps = new GraphicsPath[9];

            Graphics gg = panel.CreateGraphics();

            for (int i = 0; i < 9; i++)
                using (gps[i] = new GraphicsPath())
                {
                    gps[i].AddPolygon(points[i]);
                    gg.DrawPath(new Pen(Color.Blue, 1), gps[i]);
                }

        }
    }
}

As a side note, here is the code prior to rearrangement, where it works, it doesn't throw an exception (though it's perhaps inefficient in that every mousedown it adds polygons to determine which is clicked, when really that only needs to be done once).

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;
using System.Drawing.Drawing2D;

namespace WindowsFormsApplication15
{
    public partial class Form1 : Form
    {
        Label lbl1;
        Panel panel;
        Timer timer1;

        Point[][] points = new Point[9][];


        public Form1()
        {
            InitializeComponent();
        }

        private void Form1_Load(object sender, EventArgs e)
        {
            panel = new Panel();


            lbl1 = new Label();
            timer1 = new Timer();

            this.Controls.Add(panel);

            Label alias = lbl1;
            alias.Text = "box clicked";
            alias.Visible = false;

            panel.BackgroundImageLayout = ImageLayout.Stretch;

            this.Height = 500; this.Width = 500;


            panel.Controls.Add(lbl1);

            panel.MouseDown += p_MouseDown;
            timer1.Tick += timer1_Tick;

            panel.BackColor = Color.White;
            panel.Height = 400;
            panel.Width = 400;

            for (int i = 0; i < 9; i++) points[i] = new Point[4];

            points[0][0] = new Point(19, 262);  //left top
            points[0][1] = new Point(28, 257); //right top
            points[0][2] = new Point(27, 284);   //right bottom
            points[0][3] = new Point(16, 285);  //left bottom


            points[1][0] = new Point(52, 253);
            points[1][1] = new Point(62, 250);
            points[1][3] = new Point(49, 278);
            points[1][2] = new Point(61, 277);

            points[2][0] = new Point(87, 249);
            points[2][1] = new Point(100, 248);
            points[2][3] = new Point(86, 274);
            points[2][2] = new Point(99, 275);

            points[3][0] = new Point(126, 250);
            points[3][1] = new Point(140, 252);
            points[3][3] = new Point(126, 277);
            points[3][2] = new Point(139, 279);

            points[4][0] = new Point(164, 257);
            points[4][1] = new Point(175, 260);
            points[4][3] = new Point(164, 284);
            points[4][2] = new Point(175, 287);

            points[5][0] = new Point(197, 265);
            points[5][1] = new Point(209, 269);
            points[5][3] = new Point(198, 292);
            points[5][2] = new Point(209, 295);

            points[6][0] = new Point(228, 273);
            points[6][1] = new Point(241, 275);
            points[6][3] = new Point(229, 300);
            points[6][2] = new Point(240, 300);

            points[7][0] = new Point(262, 274);
            points[7][1] = new Point(274, 273);
            points[7][3] = new Point(262, 301);
            points[7][2] = new Point(275, 300);

            points[8][0] = new Point(297, 272);
            points[8][1] = new Point(308, 268);
            points[8][3] = new Point(298, 296);
            points[8][2] = new Point(311, 295);


            panel.Paint += thepanel_Paint;

        }


        void p_MouseDown(object sender, EventArgs e)
        {
            MouseEventArgs mea = (MouseEventArgs)e;

            GraphicsPath[] gps = new GraphicsPath[9];
            bool boxhit = false;


            for (int i = 0; i < 9; i++)
            {
                using (gps[i] = new GraphicsPath())
                using (Pen pen = new Pen(Color.Black,1))  //this pen is not for drawing. but for the logic re graphicspath
                {

                    gps[i].AddPolygon(points[i]);


                    if (gps[i].IsOutlineVisible(new Point(mea.X, mea.Y), pen) == true || gps[i].IsVisible(new Point(mea.X, mea.Y))) 
                    {
                        boxhit = true;
                        lbl1.Visible = true;
                        timer1.Enabled = true;
                    }


                }

            } //for

           // if (boxhit == false) MessageBox.Show("no box hit");


        }


        private void Form1_MouseDown(object sender, MouseEventArgs e)
        {



        }

        private void Form1_Paint(object sender, PaintEventArgs e)
        {

        }

        private void timer1_Tick(object sender, EventArgs e)
        {
            lbl1.Visible = false; timer1.Enabled = false;

        }



        private void thepanel_Paint(object sender, PaintEventArgs e)
        {
            GraphicsPath[] gps = new GraphicsPath[9];

            Graphics gg = panel.CreateGraphics();

            for (int i = 0; i < 9; i++)
                using (gps[i] = new GraphicsPath())
                {
                    gps[i].AddPolygon(points[i]);
                    gg.DrawPath(new Pen(Color.Blue, 1), gps[i]);
                }

        }
    }
}

Added

In reply to Hans's comment.

I see I missed a using there for that graphics object in the panel paint procedure. But if I add it, so, with this code.

 using(Graphics gg = panel.CreateGraphics())
            for (int i = 0; i < 9; i++)
                using (gps[i] = new GraphicsPath())
                {
                    gps[i].AddPolygon(points[i]);
                    gg.DrawPath(new Pen(Color.Black, 1), gps[i]);
                }

I still get the same exception on the cond1 = gps[i].IsOutlineVisible(new Point(mea.X, mea.Y), pen); line in the p_MouseDown procedure.

Added further

the Q is answered, but a further weird thing.. I tried to create a simple program that gives the same exception, but the program didn't crash or run correctly so maybe whether a runtime error occurs or not isn't that clearly predicatable. Here i'd expect an outofmemoryexception, or at least two messageboxes not just one, and I get no exception and just one messagebox.

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace blah
{
    public partial class Form1 : Form
    {
        Pen p;

        public Form1()
        {
            InitializeComponent();
        }

        private void Form1_Load(object sender, EventArgs e)
        {
            MessageBox.Show("sadf"); //displays
            MessageBox.Show(p.Color.ToString()); // doesn't display a messagebox at all!
        }

    }
}

You are leaking a Graphics object in the Paint event handler. Perhaps some more as well, the exceptions are flaky once the OS decides you are leaking too much and won't let you create any more. Use Task Manager, Processes tab and add the columns for GDI Objects and USER Objects so you can see the leaks adding up. — Hans Passant, Dec 26 '15 at 18:02
@HansPassant thanks.. I still get the same exception on the same line, (in p_MouseDown procedure), even when I add the `using` to the graphics object in the paint event handler. I've added a note re that to my question. — barlop, Dec 26 '15 at 19:17
@HansPassant just checked User objects and GDI objects and they're both 32 and 37 when I run it and at the time of the exception. — barlop, Dec 27 '15 at 00:03
@barlop maybe it's 'cos I declared one globally.. I guess an even better design in terms of memory would be if I had just one graphicspath and changed the points — barlop, Dec 27 '15 at 10:10
an interesting slightly related question http://stackoverflow.com/questions/34373828/why-dispose-an-object-which-will-surely-get-disposed-of-soon-regardless — barlop, Jan 15 '16 at 16:58

Mikko Viitala · Accepted Answer · 2015-12-27T14:14:34.797

Besides you are initializing a Pen and some other objects you are not using, you use using keyword to create your GraphicsPaths. So what happens when you exit the using statement... yes, object is disposed.

Instead, try creating your paths e.g.

for (int i = 0; i < 9; i++)
{
    gps[i] = new GraphicsPath();
    gps[i].AddPolygon(points[i]);
}

So this really has nothing to do with app getting OutOfMemory... but yeah, exception is misleading at least.

Using above your app works but as @Hans Passant stated, you are still leaking memory.

Edit:

Here's a re-write of your code, just "because" :). Hope it helps and/or makes sense.

using System;
using System.Collections.Generic;
using System.Drawing;
using System.Drawing.Drawing2D;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace WindowsFormsApplication1
{
    // It's preferred to use established naming conventions,
    // such as prefixing private fields with "_".

    // Declaring access modifiers explicitly, even default modifier
    // "private", is advised because it makes reading thousands lines of code
    // that much easier (one can expect first keyword to be access modifier)

    public partial class Form1 : Form
    {
        // For convenience and readability, 
        // I'd prefer Lists and List-of-Lists over arrays
        private readonly List<List<Point>> _points = new List<List<Point>>();
        private readonly List<GraphicsPath> _graphicsPaths = new List<GraphicsPath>();

        public Form1()
        {
            InitializeComponent();

            // Points to create paths from    
            _points.AddRange(new[] 
            {
                new List<Point>
                {
                    new Point(19, 62),
                    new Point(28, 57),
                    new Point(27, 84),
                    new Point(16, 85)
                },
                new List<Point>
                {
                    new Point(52, 53),
                    new Point(62, 50),
                    new Point(61, 77),
                    new Point(49, 78)
                },
                new List<Point>
                {
                    new Point(87, 49),
                    new Point(100, 48),
                    new Point(99, 75),
                    new Point(86, 74)
                }
            });

            // Create GDI graphics paths    
            foreach (List<Point> points in _points)
            {
                GraphicsPath path = new GraphicsPath();
                path.AddPolygon(points.ToArray());
                _graphicsPaths.Add(path);
            }
        }

        private void Form1_Load(object sender, EventArgs e)
        {
            // Adjust form
            Height = 200;
            Width = 100;

            // Create label and panel
            // No need for these to be private fields
            Label label = new Label
            {
                Text = @"Border clicked",
                Visible = false
            };

            // Create panel
            Panel panel = new Panel
            {
                Height = 400,
                Width = 400,
                BackColor = Color.White,
                BackgroundImageLayout = ImageLayout.Stretch
            };

            // Paint event handler.
            // Personally I prefer inline anonymous methods
            // over named methods when logic is simple 
            // and it's not being reused    
            panel.Paint += (o, args) =>
            {
                // 'using' because we want to get rid of Graphics
                // and Pen when we are done drawing paths
                using (Graphics graphics = panel.CreateGraphics())
                {
                    using (Pen pen = new Pen(Color.Blue, 3))
                    {
                        foreach (GraphicsPath path in _graphicsPaths)
                            graphics.DrawPath(pen, path);
                    }
                }
            };

            // Mouse (down) event handler.     
            panel.MouseDown += (o, args) =>
            {
                // Get mouse point
                Point mousePoint = new Point(args.X, args.Y);

                // Again, we want to dispose Pen
                using (Pen pen = new Pen(Color.Transparent, 0F))
                {
                    // Get first path under mouse pointer  
                    GraphicsPath path = _graphicsPaths.FirstOrDefault(p =>
                        p.IsOutlineVisible(mousePoint, pen));

                    if (path == null)
                        return;

                    // If found, "flash" our informative label
                    // in non-blocking way  
                    Task.Run(() =>
                    {
                        label.Invoke((Action)(() => label.Visible = true));
                        Thread.Sleep(500);
                        label.Invoke((Action)(() => label.Visible = false));
                    });
                }
            };

            // Add controls to containers 
            panel.Controls.Add(label);                
            Controls.Add(panel);
        }

        private void Form1_FormClosing(object sender, FormClosingEventArgs e)
        {
            // This could be more reasonable place to dispose
            // GDI Graphics path created earlier? 
            foreach (GraphicsPath path in _graphicsPaths)
                path.Dispose();
            _graphicsPaths.Clear();
        }
    }
}

aren't I supposed to use `using` - if I remove `using`, as you suggest, then the object isn't getting disposed. But one is supposed to dispose of IDisposable objects. — barlop, Dec 26 '15 at 19:10
also, in both the working version and the non working version, I have ` using (gps[i] = new GraphicsPath())` So it can't be as simple as "you use using keyword to create your GraphicsPaths [hence the problem]" — barlop, Dec 26 '15 at 19:51
ok I see in the form load procedure that for loop you mention works as it doesnt dispose of the object.. so it is available for the mousedown procedure. But perhaps that's bad practice to not dispose of the object(s) — barlop, Dec 27 '15 at 01:48
You dispose object when you dont need them anymore. In this context disposing does not make sense. — Mikko Viitala, Dec 27 '15 at 10:00

c# Pen throws OutOfMemoryException

1 Answers1