Should I dispose local class variables?

Question

I have a legacy application where I am getting OutOfMemory exception time to time. I am trying to debug and fix the issue. One of the option I am thinking is to use the Dispose() method. I know that any object implementing the IDisposable interface will let me call the Dispose() method on it but will it also help freeing up resources to set the class level variables to null? like ArrayList or Hashtable?

I did search on internet and found a few posts but got a bit confused whether to null objects or not.

Code I am trying

public class RepET : base_entitlement, IDisposable
{
    #region Public Member Variables
    public const int ENTITLEMENT_TYPE = 7;
    public string table = "acc_rep";
    public ArrayList country_list;
    public Hashtable data;
    public ArrayList survey_dates;
    public ArrayList city_list;
    public Dictionary<string, Dictionary<string, string>> cityData;
    public Dictionary<string, Dictionary<string, string>> reportCity_list;
    public RepET(string entId)
    {
        if (id != "0")
        {
            id = entId;
            // if we cant load the metadata then load the full records & build the metadata

            if (!LoadFromMetaDb(id, ENTITLEMENT_TYPE))
            {
                if (this.load_entitlements())
                {
                    serialize_to_metadb(this, ENTITLEMENT_TYPE);
                }
            }
        }
    }
    #endregion

    public bool load_entitlements()
    {
        //  loads this entitlement from the metadata
        this.data = new Hashtable();
        this.survey_dates = new ArrayList();
        this.city_list = new ArrayList();
        //SqlQueries.ProcessingStarted(id);
        var sdates = SqlQueries.GetNewestSurveyByET(this.table, id, "acc_rep");

        if (sdates.Count == 0) return false;
        else
        {
            //delete ent 4 if already exist as its regenerated here
            check_EntFile_Created(id, Constants.ENTITLEMENT_TYPE_RP.ToString(), true);
            o_report_entitlements = new report_entitlements(id);

            reportCity_list = new Dictionary<string, Dictionary<string, string>>();
            foreach (string sd in sdates)
            {
                ent_by_survey_date(sd.Trim(), true);
                this.survey_dates.Add(sd.Trim());
            }
            o_report_entitlements.serialize_to_metadb(o_report_entitlements, Constants.ENTITLEMENT_TYPE_RP);

            return true;
        }
    }

    public bool ent_by_survey_date(string survey_date, bool modify_report_entitlements = false)
    {

        //if (modify_report_entitlements) {
        //    ArrayList countryArray;
        //}

        var dt = SqlQueries.Ent_by_survey_date(table, id, survey_date, "acc");
        if (dt.Rows.Count == 0)
        {
            return false;
        }
        else
        {
            country_list = new ArrayList();
            city_list = new ArrayList();

            Dictionary<string, ArrayList> countryCodes = new Dictionary<string, ArrayList>();

            foreach (DataRow row in dt.Rows)
            {
                string current_city_code = row["city_code"].ToString().Trim();
                string current_report_type = row["report_type"].ToString().Trim();

                if (!string.IsNullOrEmpty(current_city_code))
                {
                    Dictionary<string, string> currentCityReportList = new Dictionary<string, string>();
                    if (!reportCity_list.ContainsKey(survey_date))
                    {
                        reportCity_list[survey_date] = new Dictionary<string, string>();
                        reportCity_list[survey_date].Add(current_city_code, current_report_type);
                    }
                    else if (reportCity_list != null && reportCity_list.ContainsKey(survey_date) && !reportCity_list[survey_date].ContainsKey(current_city_code))
                    {
                        reportCity_list[survey_date].Add(current_city_code, current_report_type);
                    }

                    if (modify_report_entitlements)
                    {

                        string current_country_code = get_country_code_by_city(current_city_code);

                        if (!country_list.Contains(current_country_code))
                        {
                            country_list.Add(current_country_code);
                            foreach (var item in ((System.Reflection.TypeInfo)(o_report_entitlements.GetType())).DeclaredFields)
                            {
                                if (item.Name == "data")
                                {
                                    Hashtable tempObj = (Hashtable)item.GetValue(o_report_entitlements);
                                    if (tempObj != null)
                                    {
                                        countryCodes = (Dictionary<string, ArrayList>)tempObj[id];
                                        if (countryCodes != null && !countryCodes.ContainsKey(current_country_code))
                                            this.o_report_entitlements.add_to_array(current_country_code, "ACC", "", ref countryCodes);
                                        else if (countryCodes != null && countryCodes.ContainsKey(current_country_code) && !countryCodes[current_country_code].Contains("ACC"))
                                            this.o_report_entitlements.add_to_array(current_country_code, "ACC", "", ref countryCodes);
                                        else if (!countryCodes.ContainsKey(current_country_code))
                                            this.o_report_entitlements.add_to_array(current_country_code, "ACC", "", ref countryCodes);
                                    }
                                    else
                                    {
                                        if (countryCodes == null)
                                            countryCodes = new Dictionary<string, ArrayList>();
                                        this.o_report_entitlements.add_to_array(current_country_code, "ACC", "", ref countryCodes);
                                    }
                                }
                            }
                        }
                    }

                    if (!city_list.Contains(current_city_code))
                    {
                        city_list.Add(current_city_code);
                    }
                    if (!currentCityReportList.ContainsKey(current_city_code))
                    {
                        currentCityReportList.Add(current_city_code, current_report_type);
                    }
                    if (!data.ContainsKey(survey_date))
                    {
                        data[survey_date] = new Hashtable();
                    }
                    switch ((((Hashtable)this.data[survey_date])).GetType() == typeof(Dictionary<string, string>))
                    {
                        case true:
                            break;
                        case false:
                        default:
                            ((Hashtable)this.data[survey_date])[current_city_code] = new Dictionary<string, string>();
                            (((Dictionary<string, string>)((Hashtable)this.data[survey_date])[current_city_code])["city_code"]) = current_city_code;
                            //(((Dictionary<string, string>)((Hashtable)this.data[survey_date])[current_city_code])["city_code"])


                            //(((Dictionary<string, ArrayList>)((Hashtable)this.data[survey_date])[current_city_code])["report_type"]) = new ArrayList();
                            (((Dictionary<string, string>)((Hashtable)this.data[survey_date])[current_city_code])["report_type"]) = current_report_type;
                            //((Dictionary<string, string>)((Hashtable)this.data[survey_date])[current_city_code])["report_type"] = current_report_type;
                            break;
                    }
                }
            }
        }
        return true;
    }

    public string get_country_code_by_city(string city_code)
    {

        load_city_list();
        string returnItem = null;
        foreach (var item in cityData)
        {
            Dictionary<string, string> subItem = item.Value;
            if (subItem.ContainsKey(city_code))
            {
                returnItem += item.Key;
            }
        }
        return returnItem;
    }

    public bool load_city_list()
    {
        if (GlobalObjs.dtCityList.Rows.Count == 0)
            GlobalObjs.dtCityList = SqlQueries.LoadCityList();

        //var dt = SqlQueries.LoadCityList();
        if (GlobalObjs.dtCityList.Rows.Count == 0)
        {
            return false;
        }
        Dictionary<string, string> cityList = new Dictionary<string, string>();
        cityData = new Dictionary<string, Dictionary<string, string>>();
        foreach (DataRow row in GlobalObjs.dtCityList.Rows)
        {
            string Country_Code = row["Country_Code"].ToString().Trim();
            string City_Code = row["City_Code"].ToString().Trim();
            string City_Name = row["Name"].ToString().Trim();

            if (!cityData.ContainsKey(Country_Code))
            {
                cityData.Add(Country_Code, new Dictionary<string, string>());
            }

            Dictionary<string, string> tempList = cityData[Country_Code];
            if (!tempList.ContainsKey(City_Code))
            {
                tempList.Add(City_Code, City_Name);
                //cityList.Add(City_Code, City_Name);
                cityData[Country_Code] = tempList;
            }

        }
        return true;
    }



    // Flag: Has Dispose already been called?
    bool disposed = false;

    // Protected implementation of Dispose pattern.
    protected void Dispose(bool disposing)
    {
        if (disposed)
            return;

        if (disposing)
        {
            country_list = null;
            data = null;
            survey_dates = null;
            city_list = null;
            cityData = null;
            reportCity_list = null;
        }

        disposed = true;
    }
    public void Dispose()
    {
        Dispose(true);
    }
}

will this help with the memory exception?

no, setting internal class members won't help with OutOfMemoryErrors. GC is pretty smart to free memory occupied by `RepET` properties. The problem is that something holds references (like static event handlers) so you have to find the memory leak. You can use in-built VS memory profiler snapshots to check the difference between snapshots and try to find object are not being removed by GC. Do you dispose `RepET.connection` property BTW ? — oleksa, Mar 03 '20 at 09:41
FYI, you shouldn't use `ArrayList`, use `List` ([source](https://learn.microsoft.com/en-us/dotnet/api/system.collections.arraylist#remarks)). I would also avoid `Hashtable` if possible (see [this question](https://stackoverflow.com/questions/301371/why-is-dictionary-preferred-over-hashtable-in-c)) — Joelius, Mar 03 '20 at 09:42
@Rohit post the *rest* of the code. Typically OOMs are caused by memory fragmentation caused by trying to insert eg 1M items into a list without specifying an initial capacity. The quick fix is to specify a capacity for the container. The *real* fix is to just *not* load all that data in the first place — Panagiotis Kanavos, Mar 03 '20 at 09:44
@Rohit this means that the methods in the question's code probably aren't relevant. The *relevant* code is the code that creates and loads those container fields (they aren't variables). Specifying a capacity when creating the containers could fix the problem — Panagiotis Kanavos, Mar 03 '20 at 09:46
@PanagiotisKanavos thanks for the suggestion, what do you mean by specifying capacity? — Rohit, Mar 03 '20 at 09:58
@Rohit I mean that you should post your code. When you create any container you can specify a capacity but *why are you loading so many items* in the first place? Whatever your code does, it doesn't have to store 1M items in memory. — Panagiotis Kanavos, Mar 03 '20 at 10:00
You are using various DataTables and such. What is the size of them (number of rows, number of columns)? — Hans Keﬆing, Mar 03 '20 at 10:28

score 4 · Answer 1 · answered Mar 03 '20 at 09:42

The only time setting fields to null will make a difference to garbage collection is if you're still somehow leaving the RepET instance reachable in some way, in which case the real solution is: make sure that the RepET instance is no longer reachable! Setting the fields to null is papering over the real problem.

It is appropriate to use Dispose() to cascade things like connection and cmd, but... another part of me thinks the better approach there would be to not store them, i.e. obtain the connection on-demand, and keep cmd entirely local to where it is used; something like:

using (var conn = SomeHelper.CreateConnection())
{
    // your "cmd" code in here, via "using" - or
    // perhaps via "Dapper" and let it worry about that
}

The real trick, though, is finding what is holding the instance, if that is the real problem. Hard to comment without context, but: events and static caches are good places to start looking.

I also though of updating the DB connection code to use `using` thanks for suggestion. I'll change it in code. — Rohit, Mar 03 '20 at 10:13

Raju Dasupally · Answer 2 · 2020-03-03T11:07:48.260

2

As per the Microsoft documentation, the OutOfMemory exception is thrown when CLR can not allocate contiguous memory to perform an operation. In your case, since you are using Array-based data structures which may need contiguous memory allocation to a lot of data, hence you are getting the error.

- [https://learn.microsoft.com/en-us/dotnet/api/system.outofmemoryexception?view=netframework-4.8][1]

Instead of loading all the data at once into these array-based data structures, you may want to load as much less data as required for processing or try lazy loading

edited Mar 03 '20 at 11:07

answered Mar 03 '20 at 09:57

Raju Dasupally

164
7

That is a reasonable *hypothesis* that warrants investigation; I would be hesitant to presume that it is the *explanation*; I've seen OOMs from far more subtle and nuanced things than simply oversized arrays. But yes: oversized arrays should be avoided. – Marc Gravell Mar 03 '20 at 10:21
Thanks, @HansKeﬆing for the correction! updated the spellings. – Raju Dasupally Mar 03 '20 at 11:09

Should I dispose local class variables?

2 Answers2