patterncsharpMinor
Memory leak using UserControls
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.
Timer (10sec):
If I leave the application
//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
It will allow you to get rid of those ugly private variables in exchange for fun scope specific variables in the
Here is what I would do:
Your
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.