HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Memory leak using UserControls

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
usingleakmemoryusercontrols

Problem

I seem to have a memory leak on this method but can't figure it out. This method is called every 10sec by a timer which acts like a refresh.

//Variables

    List theDataPages = new List();

    private OleDbConnection mycon;
    private OleDbCommand oleDbCmd;
    private OleDbDataReader dataReader;

    //This method looks for the Alerts in the Data Base and displays the info in a User Control 
    private void ShowData()
    {
        try
        {

            mycon.Open();

            string queriesLabels = "SELECT *FROM MP where State = @stateAccepted OR State = @stateRejected";

            oleDbCmd = new OleDbCommand(queriesLabels, mycon);                
            oleDbCmd.Parameters.Add("@stateAccepted", OleDbType.VarChar, 10).Value = "Accepted";
            oleDbCmd.Parameters.Add("@stateRejected", OleDbType.VarChar, 10).Value = "Rejected";

            dataReader = oleDbCmd.ExecuteReader();              
                while (dataReader.Read())
                {

                    TabPage page = new TabPage("MP "
                                 + (DTL.TabPages.Count + 1).ToString());
                    UserControls.userControlAlert aForm = new UserControls.userControlAlert();
                    aForm.Parent = page;
                    aForm.showReaderRow(dataReader);
                    DTL.TabPages.Add(page);
                    theDataPages.Add(aForm);
                }  
        }
        catch (Exception error)
        {              
            MessageBox.Show(error.ToString());
        }
        finally
        {
            dataReader.Close();
            mycon.Close();
        }
    }


Timer (10sec):

private void timer1_Tick(object sender, EventArgs e)
    {
        if (DTL.TabCount > 0)
        {
            theDataPages.Clear();
            DTL.TabPages.Clear();
            ShowData();
        }
        else
        {
            theDataPages.Clear();
            ShowData();
        }
    }


If I leave the application

Solution

Your code should make use of using statements instead of the finally statement of the try/catch.

It will allow you to get rid of those ugly private variables in exchange for fun scope specific variables in the using statements. And no matter what the connections are all closed. I think that you are leaving open the OldDbCommand which might be holding onto the OleDbConnection or at the very least just holding the whole command in memory, but when it runs again it creates an entirely new command.

private OleDbConnection mycon;
private OleDbCommand oleDbCmd;
private OleDbDataReader dataReader;

//This method looks for the Alerts in the Data Base and displays the info in a User Control 
private void ShowData()
{
    try
    {

        mycon.Open();

        string queriesLabels = "SELECT *FROM MP where State = @stateAccepted OR State = @stateRejected";

        oleDbCmd = new OleDbCommand(queriesLabels, mycon);                
        oleDbCmd.Parameters.Add("@stateAccepted", OleDbType.VarChar, 10).Value = "Accepted";
        oleDbCmd.Parameters.Add("@stateRejected", OleDbType.VarChar, 10).Value = "Rejected";

        dataReader = oleDbCmd.ExecuteReader();              
            while (dataReader.Read())
            {

                tabPage page = new tabPage("MP "
                             + (DTL.TabPages.Count + 1).ToString());
                UserControls.userControlAlert aForm = new UserControls.userControlAlert();
                aForm.Parent = page;
                aForm.showReaderRow(dataReader);
                DTL.TabPages.Add(page);
                theDataPages.Add(aForm);
            }  
    }
    catch (Exception error)
    {              
        MessageBox.Show(error.ToString());
    }
    finally
    {
        dataReader.Close();
        mycon.Close();
    }
}


Here is what I would do:

private void ShowData()
{
    try
    {
        using (OleDbConnection mycon = new OleDbConnection("connectionString")
        {
            mycon.Open();
            string queriesLabels = "SELECT *FROM MP where State = @stateAccepted OR State = @stateRejected";

            using (OleDbCommand oleDbCmd = new OleDbCommand(queriesLabels, mycon))
            using (oleDbDataReader dataReader = oleDbCmd.ExecuteReader())
            {
                oleDbCmd.Parameters.Add("@stateAccepted", OleDbType.VarChar, 10).Value = "Accepted";
                oleDbCmd.Parameters.Add("@stateRejected", OleDbType.VarChar, 10).Value = "Rejected";

                while (dataReader.Read())
                {
                    tabPage page = new tabPage("MP "
                                + (DTL.TabPages.Count + 1).ToString());
                    UserControls.userControlAlert aForm = new UserControls.userControlAlert();
                    aForm.Parent = page;
                    aForm.showReaderRow(dataReader);
                    DTL.TabPages.Add(page);
                    theDataPages.Add(aForm);
                }  
            }
        }
    }
    catch (Exception error)
    {              
        MessageBox.Show(error.ToString());
    }
}


Your OleDbCommand makes use of the IDisposable interface, so use it and make sure that it doesn't leak.

Code Snippets

private OleDbConnection mycon;
private OleDbCommand oleDbCmd;
private OleDbDataReader dataReader;

//This method looks for the Alerts in the Data Base and displays the info in a User Control 
private void ShowData()
{
    try
    {

        mycon.Open();

        string queriesLabels = "SELECT *FROM MP where State = @stateAccepted OR State = @stateRejected";

        oleDbCmd = new OleDbCommand(queriesLabels, mycon);                
        oleDbCmd.Parameters.Add("@stateAccepted", OleDbType.VarChar, 10).Value = "Accepted";
        oleDbCmd.Parameters.Add("@stateRejected", OleDbType.VarChar, 10).Value = "Rejected";

        dataReader = oleDbCmd.ExecuteReader();              
            while (dataReader.Read())
            {

                tabPage page = new tabPage("MP "
                             + (DTL.TabPages.Count + 1).ToString());
                UserControls.userControlAlert aForm = new UserControls.userControlAlert();
                aForm.Parent = page;
                aForm.showReaderRow(dataReader);
                DTL.TabPages.Add(page);
                theDataPages.Add(aForm);
            }  
    }
    catch (Exception error)
    {              
        MessageBox.Show(error.ToString());
    }
    finally
    {
        dataReader.Close();
        mycon.Close();
    }
}
private void ShowData()
{
    try
    {
        using (OleDbConnection mycon = new OleDbConnection("connectionString")
        {
            mycon.Open();
            string queriesLabels = "SELECT *FROM MP where State = @stateAccepted OR State = @stateRejected";

            using (OleDbCommand oleDbCmd = new OleDbCommand(queriesLabels, mycon))
            using (oleDbDataReader dataReader = oleDbCmd.ExecuteReader())
            {
                oleDbCmd.Parameters.Add("@stateAccepted", OleDbType.VarChar, 10).Value = "Accepted";
                oleDbCmd.Parameters.Add("@stateRejected", OleDbType.VarChar, 10).Value = "Rejected";

                while (dataReader.Read())
                {
                    tabPage page = new tabPage("MP "
                                + (DTL.TabPages.Count + 1).ToString());
                    UserControls.userControlAlert aForm = new UserControls.userControlAlert();
                    aForm.Parent = page;
                    aForm.showReaderRow(dataReader);
                    DTL.TabPages.Add(page);
                    theDataPages.Add(aForm);
                }  
            }
        }
    }
    catch (Exception error)
    {              
        MessageBox.Show(error.ToString());
    }
}

Context

StackExchange Code Review Q#79797, answer score: 4

Revisions (0)

No revisions yet.