3

I have developed a Point of Sale system using C#, Winforms and Mysql. After its deployment, I observed that the memory size keeps on increasing with time. After evaluating my code, I felt my Data Layer might be the culprit. I have generalized database call using methods like these

 public static DataTable ExecQuery(string query, List<SqlParam> sp_params, string db)
 {

        MySqlConnection sCon = new MySqlConnection();
        sCon.ConnectionString = "server=" + server + ";port=" + port + ";database=" + db + ";uid=" + user + ";pwd=" + password + ";charset=utf8;";


        MySqlCommand command = new MySqlCommand();
        command.CommandType = CommandType.Text;
        command.Connection = sCon;

        if (sp_params != null)
        {
            for (int i = 0; i < sp_params.Count; i++)
            {
                MySqlParameter sparam = new MySqlParameter();
                sparam.ParameterName = sp_params[i].Name;
                sparam.MySqlDbType = sp_params[i].Type;
                sparam.Value = sp_params[i].Value;
                command.Parameters.Add(sparam);
            }
        }
        command.CommandText = query;
        sCon.Open();
        MySqlDataReader sd = command.ExecuteReader();



        DataTable dt = new DataTable();

        for (int fc = 0; fc < sd.FieldCount; fc++)
        {
            if (dt.Columns.Contains(sd.GetName(fc)))
            {
                dt.Columns.Add(sd.GetName(fc) + "1", sd.GetFieldType(fc));
            }
            else
            {
                dt.Columns.Add(sd.GetName(fc), sd.GetFieldType(fc));
            }
        }

        while (sd.Read())
        {
            DataRow dr = dt.NewRow();
            for (int fc = 0; fc < sd.FieldCount; fc++)
            {
                dr[fc] = sd.GetValue(fc);
            }
            dt.Rows.Add(dr);
        }

        sCon.Close();
        return dt;

    }

For every database call, We use this method. The front end has to only specify parameters and query. Am I right in my suspicion that this static method is causing memory problems?

Update:

Just found another leak in my App:

Wherever reportviewer is used, LocalReport.ReleaseSandboxAppDomain() must be called in host form's formclosing event. Otherwise each invocation of Report, adds to memory size.

Update 2

Found the actual cause of Memory Leak in my system. I am Using a complex usercontrol which I add in a flowlayoutpanel.

I was using a normal foreach loop to dispose each control.. but somehow only half objects were disposed. I nested this foreach loop inside a for loop with counter as number of controls.

        int count = flwControls.Controls.Count;
        for (int i = 0; i < count; i++)
        {
            foreach (Control c in flwControls.Controls)
            {
                c.Dispose();
            }
        }
  • @JoeDF is the static'ness' of method causing the leak? or any particular part of the code – Akshay Zadgaonkar Sep 13 '14 at 13:39
  • 1
    The function looks OK (aside from missing any exception handling or a using/finally clause to force release of resources). Is it possible the table you allocate and return ( `DataTable dt = new DataTable();` ) is never being released? – Steve Wellens Sep 13 '14 at 13:42
  • You'd be well served releasing the command, data reader and data table objects in a finally block. – Rahul Vijay Dawda Sep 13 '14 at 13:50
  • @stevewellens this data table ends up being data source for winform controls.. is it released when control is disposed – Akshay Zadgaonkar Sep 13 '14 at 13:56
  • Does C# allow connection creation without exception handling? If not, I would suggest you to enclose your connections in a try-catch and close it in the finally block. – Pratyay Sep 13 '14 at 13:57
  • @pratyay I agree i should do that. But mysql connection count at server end remains consistent with calls.. still the memory usage remains high – Akshay Zadgaonkar Sep 13 '14 at 14:01

4 Answers4

2

SqlConnection, SqlCommand and SqlDataReader are all IDisposable. The first two are in addition sealed. Assuming your MySqlXXX classes are encapsulating these classes, you need to make them be disposable by implementing the basic dispose pattern, and dispose of them by wrapping them in a using statement.

DataTable is also disposable, so be sure to dispose of it after it is used higher up in your code.

Here is an example of how to dispose of these resources. (Note I can't test because I don't have the definition for the MySqlXXX classes):

    public static DataTable ExecQuery(string query, List<SqlParam> sp_params, string db)
    {
        using (MySqlConnection sCon = new MySqlConnection())
        using (MySqlCommand command = new MySqlCommand())
        {
            sCon.ConnectionString = "server=" + server + ";port=" + port + ";database=" + db + ";uid=" + user + ";pwd=" + password + ";charset=utf8;";
            command.CommandType = CommandType.Text;
            command.Connection = sCon;

            if (sp_params != null)
            {
                for (int i = 0; i < sp_params.Count; i++)
                {
                    MySqlParameter sparam = new MySqlParameter();
                    sparam.ParameterName = sp_params[i].Name;
                    sparam.MySqlDbType = sp_params[i].Type;
                    sparam.Value = sp_params[i].Value;
                    command.Parameters.Add(sparam);
                }
            }
            command.CommandText = query;
            sCon.Open();
            using (MySqlDataReader sd = command.ExecuteReader())
            {
                DataTable dt = new DataTable();
                for (int fc = 0; fc < sd.FieldCount; fc++)
                {
                    if (dt.Columns.Contains(sd.GetName(fc)))
                    {
                        dt.Columns.Add(sd.GetName(fc) + "1", sd.GetFieldType(fc));
                    }
                    else
                    {
                        dt.Columns.Add(sd.GetName(fc), sd.GetFieldType(fc));
                    }
                }

                while (sd.Read())
                {
                    DataRow dr = dt.NewRow();
                    for (int fc = 0; fc < sd.FieldCount; fc++)
                    {
                        dr[fc] = sd.GetValue(fc);
                    }
                    dt.Rows.Add(dr);
                }
                return dt;
            }
        }
    }
}

Update

In answer to the question below, I tested and found that disposing of a DataGridView or changing the DataSource does not automatically dispose of the previous DataSource - perhaps since the DataSource is typed only as an object. You can do it yourself like so:

public partial class Form1 : Form
{
    public Form1()
    {
        InitializeComponent();
        dataGridView1.Disposed += dataGridView_Disposed;
    }

    static void dataGridView_Disposed(object sender, EventArgs e)
    {
        var dataGridView = sender as DataGridView;
        if (dataGridView != null)
        {
            var oldTable = dataGridView.DataSource as IDisposable;
            if (oldTable != null)
                oldTable.Dispose();
        }
    }

    private void FillDataGridView(object sender, EventArgs e)
    {
        var oldTable = dataGridView1.DataSource as IDisposable;
        DataTable table = GenerateTable();
        dataGridView1.DataSource = table;
        if (oldTable != null)
            oldTable.Dispose();
    }
}
dbc
  • 104,963
  • 20
  • 228
  • 340
  • Thank you very much for an apt and code based solution. My concern here is does the data table acting as data source dispose itself when the parent control like datagridview is disposed – Akshay Zadgaonkar Sep 13 '14 at 14:51
  • @Akshay Zadgaonkar - you would need to show us the code where you think the leak may be. Or at least, a [minimal, complete and verifiable](http://stackoverflow.com/help/mcve) example. – dbc Sep 13 '14 at 14:54
  • @Akshay Zadgaonkar - that being said, perhaps this is relevant: http://stackoverflow.com/questions/16238206/where-is-my-memory-re-initializing-datatable – dbc Sep 13 '14 at 14:59
  • Disposing objects will not cure memory leaks. Those are two different aspects of .NET. – Steve Wellens Sep 14 '14 at 00:13
1

You have a memory leak. After you close scon, you need to free it.

phil
  • 561
  • 3
  • 10
1

The code looks fine in terms of memory leak.

It may be possible that the other code of your application cause the memory leak. E.g. The DataTable returned by this code, stays on memory?

One suggestion for above code to use pooling=true for better performance.

Naresh Goradara
  • 1,896
  • 2
  • 30
  • 41
0

Use Timer instead of a loop (put all code in a loop to timer tick function). Or put all your code in timer tick function.