Let's play 'Can you spot the memory leak?'

I noticed yesterday that if I left my application on the Spider 2 running for 10-20 minutes I would eventually get an ‘Out of memory’ error. I bypassed most of the code and selectively enabled section after section while watching the GC dump in the debug window. The memory leak shows up as the amount of memory used growing by 3-4K between each GC dump.

After a few hours of enabling more sections of code and monitoring memory usage I found the issue was with the method that decodes the incoming packets (passed as strings). I have pasted the code below as you can see it has been modified a bit to let it run without the line marked //*** The memory leak is here ***///. I let this run all night and the memory usage stayed within a few K range. Perhaps the issue is how I’m passing data to my ‘getInt’ method? The problem is not immediately clear to me.

/// <summary>
        /// Deocde RXd data packet, update local values
        /// We read everything as strings
        /// </summary>
        /// <param name="packet"></param>
        /// <returns>True if good packet</returns>
        // Decode Tx packet from Mach 4
        //       axis positions -> ":M0=f:M1=f:M2=f:M3=f:"   -> machAxisPos
        //              outputs -> ":O0=b:O1=b;O2=b...O7=b:" -> machOutputs
        // (0-3 output) general -> ":G0=b:G1=b:G2=b:G3=b:"   -> machGeneral
        private bool decodeRXpacket(string packet)
        {
            bool value = false;
            int lastDelim = packet.LastIndexOf(':');
            string dataSt = packet.Substring(0, lastDelim); // trim ending delim
            uint crc = uint.Parse(packet.Substring(lastDelim + 1, packet.Length - lastDelim - 1));
            //uint crc32 = GHI.Utilities.Crc.Crc32(Encoding.UTF8.GetBytes(dataSt), 0);

            // error if does not start/end with ':'
            //if (packet[0] == ':' && packet[packet.Length - 1] == ':')
           // if (crc == crc32)
            if (true)
            {
                value = true;

                string[] data = packet.Split(':');
                foreach (string word in data)
                {
                    if (word.Length > 3) // a valid word has to have at least 4 characters
                    {
                        string[] nameValue = word.Split('=');
                        char nameType = nameValue[0][0];
                        int nameIndex = 0;//int.Parse(nameValue[0].Substring(1, nameValue[0].Length-1));

                        //*** The memory leak is in this line ***//
                        //bool valOK = getInt(nameValue[0].Substring(1, nameValue[0].Length - 1), ref nameIndex);

                        //if (valOK)
                        if (true)
                        {
                            if (nameType == 'M' && (nameIndex >= 0 && nameIndex < 8))
                            {
                                // handles machAxisPos
                                machAxisPos[nameIndex] = nameValue[1];
                            }
                            else if (nameType == 'O' && (nameIndex >= 0 && nameIndex < 8))
                            {
                                // handles machOutputs
                                machOutputs[nameIndex] = nameValue[1];
                            }
                            else if (nameType == 'G' && (nameIndex >= 0 && nameIndex < 4))
                            {
                                // handles machGeneral
                                machGeneral[nameIndex] = nameValue[1];
                            }
                        }
                    }
                }
            }

            return value;
        }

        /// <summary>
        /// Helper to parse string to int and catch errors
        /// </summary>
        /// <param name="sValue">string to parse</param>
        /// <param name="iValue">ref to int result </param>
        /// <returns>True if no erros</returns>
        private bool getInt(string sValue, ref int iValue )
        {
            try
            {
                iValue = int.Parse(sValue);
                return true;
            }
            catch 
            {
                return false;
            }

        }
1 Like

I was able to do some more testing this morning while setting up some other things. I was able to track down the problem to this bit of code:



I tried the int.Parse directly, bypassing my getInt method and still had the memory leak. So then I fixed the value of nameIndexStr to "0" and did int.Parse on that and it works fine, no memory leak.


```cs
                    if (word.Length > 3) // a valid word has to have at least 4 characters
                    {
                        string[] nameValue = word.Split('=');
                        char nameType = nameValue[0][0];
                        string nameIndexStr = "0";//nameValue[0].Substring(1, nameValue[0].Length-1);
                        int nameIndex = int.Parse(nameIndexStr);

                        //*** The memory leak is in this line ***//
                        //bool valOK = getInt(nameValue[0].Substring(1, nameValue[0].Length - 1), ref nameIndex);

So pulling the substring out of the string array is the issue but I’m not sure why.

After a lot more testing I wound up with the code below. It seems my passing an int by reference was causing the problem. I would up wrapping the whoel section of code containing the uint.parse calls with a try/catch. I’m still unclear why passing by reference was causing a problem.

        /// <summary>
        /// Deocde RXd data packet, update local values
        /// We read everything as strings
        /// </summary>
        /// <param name="packet"></param>
        /// <returns>True if packet CRC matches</returns>
        // Decode Tx packet from Mach 4
        //       axis positions -> ":M0=f:M1=f:M2=f:M3=f:"   -> machAxisPos
        //              outputs -> ":O0=b:O1=b;O2=b...O7=b:" -> machOutputs
        // (0-3 output) general -> ":G0=b:G1=b:G2=b:G3=b:"   -> machGeneral
        private bool decodeRXpacket(string packet)
        {
            bool value = false; // return true if CRC matches
            int lastDelim = packet.LastIndexOf(':');
            string crcSt = packet.Substring(lastDelim + 1, packet.Length- lastDelim -1);
            string dataSt = packet.Substring(0, lastDelim); // trims ending delim too

            try
            {
                uint crc = uint.Parse(crcSt);
                uint crc32 = GHI.Utilities.Crc.Crc32(Encoding.UTF8.GetBytes(dataSt), 0);

                if (crc == crc32)
                {
                    value = true;
                    string[] data = dataSt.Split(':');
                    foreach (string word in data)
                    {
                        if (word.Length > 3) // a valid word has to have at least 4 characters
                        {
                            string[] name_Value = word.Split('=');
                            char nameType = name_Value[0][0];
                            string nameIndexStr = name_Value[0];
                            uint nameIndex = uint.Parse(nameIndexStr.Substring(1, nameIndexStr.Length - 1));

                            if (nameType == 'M' && (nameIndex >= 0 && nameIndex < 8))
                            {
                                machAxisPos[nameIndex] = name_Value[1];
                            }
                            else if (nameType == 'O' && (nameIndex >= 0 && nameIndex < 8))
                            {
                                machOutputs[nameIndex] = name_Value[1];
                            }
                            else if (nameType == 'G' && (nameIndex >= 0 && nameIndex < 4))
                            {
                                machGeneral[nameIndex] = name_Value[1];
                            }
                        }
                    }
                }
            }
            catch 
            { 
                // we got here most likley from an error with uint.Parse
                Debug.Print("Error decoding RX packet.");
            }

            return value;
        }