Memory leak using UserControls

Question

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<UserControls.userControlAlert> theDataPages = new List<UserControls.userControlAlert>();
 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 open, every 10sec I see an increment on the memory in that process, after some time windows closes the application.

• Jamal: I've restored the original version of the code since it was modified after receiving an answer.
• Dan Lyons: @MoralesJosue If I am right in assuming that tabPage is really a System.Windows.Forms.TabPage and UserControls.userControlAlert is some subclass of System.Windows.Forms.UserControl, then you need to dispose of those when you clear theDataPages and DTL.TabPages.
• Malachi: @DanLyons would it be safe to say that we need more code to determine how bad the leak is? do you feel this is off-topic?
• Dan Lyons: @Malachi Yes, I think we need some more code to be sure. I am also inclined to think this question might be better for SO, since it's asking to fix a problem rather than simply asking for a review of code and just happening to have leaks.
• MoralesJosue: @DanLyons , I tried using this code to dispose the TabPages "for (int i = DTL.TabCount - 1; i > -1; i--) { DTL.TabPages[i].Dispose(); }" I still have some leaks, I will post the full code in SO, and end this tread.

Answer 1

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.

• MoralesJosue: Changed all the statements to "Using" in every part of the project, still having some leaks, I'm pretty sure is in the method I posted since its the only one that the timer is using, and the increment of memory happens every time the timer event triggers. @Malachi, my guess is that it is deleting the tabs but not the object, I'm currently working on a solution on that.
• Malachi: regardless you should be using using statements.
• MoralesJosue: I will certainly do. @Malachi
• Heslacher: @MoralesJosue If you e.g store the dataReader of aForm.showReaderRow(dataReader); in a variable of aForm it won't get closed.
• MoralesJosue: @Heslacher so I should be using aForm.Dipose(); inside my timer ?