Merge pull request #13609 from ghalliday/issue23840

HPCC-23840 Fix various memory leaks in roxie

Reviewed-By: Jake Smith <jake.smith@lexisnexis.com>
Reviewed-By: Mark Kelly <mark.kelly@lexisnexis.com>
Reviewed-By: Richard Chapman <rchapman@hpccsystems.com>

ecl/hql/hqlutil.cpp

@@ -10288,6 +10288,18 @@ static IFieldFilter * createIfBlockFilter(IRtlFieldTypeDeserializer &deserialize
     return extractor.createSingleFieldFilter(deserializer);
 }
 
+static void cleanupFields(unsigned num, const RtlFieldInfo * * fieldsArray)
+{
+    for (unsigned i = 0; i < num; i++)
+    {
+        const RtlFieldInfo * child = fieldsArray[i];
+
+        if (!isVirtualInitializer(child->initializer))
+            free((void *)child->initializer);
+        delete child;
+    }
+    delete [] fieldsArray;
+}
 
 unsigned buildRtlRecordFields(IRtlFieldTypeDeserializer &deserializer, unsigned &idx, const RtlFieldInfo * * fieldsArray, IHqlExpression *record, IHqlExpression *rowRecord)
 {
@@ -10322,10 +10334,18 @@ unsigned buildRtlRecordFields(IRtlFieldTypeDeserializer &deserializer, unsigned
                 unsigned numFields = getFlatFieldCount(record);
                 info.fieldsArray = new const RtlFieldInfo * [numFields+1];
                 unsigned idx = 0;
-                info.fieldType |= buildRtlRecordFields(deserializer, idx, info.fieldsArray, record, record);
-                info.fieldsArray[idx] = nullptr;
-
-                info.filter = createIfBlockFilter(deserializer, rowRecord, field);
+                try
+                {
+                    info.fieldType |= buildRtlRecordFields(deserializer, idx, info.fieldsArray, record, record);
+                    info.fieldsArray[idx] = nullptr;
+                    info.filter = createIfBlockFilter(deserializer, rowRecord, field);
+                }
+                catch (...)
+                {
+                    //Clean up all the fields that have been added (idx is incremented by buildRtlRecordFields)
+                    cleanupFields(idx, info.fieldsArray);
+                    throw;
+                }
 
                 type = deserializer.addType(info, key);
             }
@@ -10458,7 +10478,15 @@ const RtlTypeInfo *buildRtlType(IRtlFieldTypeDeserializer &deserializer, ITypeIn
             unsigned numFields = getFlatFieldCount(record);
             info.fieldsArray = new const RtlFieldInfo * [numFields+1];
             unsigned idx = 0;
-            info.fieldType |= buildRtlRecordFields(deserializer, idx, info.fieldsArray, record, record);
+            try
+            {
+                info.fieldType |= buildRtlRecordFields(deserializer, idx, info.fieldsArray, record, record);
+            }
+            catch (...)
+            {
+                cleanupFields(idx, info.fieldsArray);
+                throw;
+            }
             info.fieldsArray[idx] = nullptr;
             break;
         }

roxie/ccd/ccdqueue.cpp

@@ -1282,6 +1282,8 @@ public:
         {
             if (E->errorCode()!=ROXIE_ABORT_ERROR)
                 throwRemoteException(E, activity, packet, false);
+            else
+                E->Release();
         }
         catch (...)
         {

roxie/ccd/ccdserver.cpp

@@ -12240,7 +12240,9 @@ class CRoxieServerIndexWriteActivity : public CRoxieServerInternalSinkActivity,
         {
             StringBuffer name(nameLen, nameBuff);
             StringBuffer value(valueLen, valueBuff);
-            if(*nameBuff == '_' && strcmp(name, "_nodeSize") != 0)
+            rtlFree(nameBuff);
+            rtlFree(valueBuff);
+            if(*name == '_' && strcmp(name, "_nodeSize") != 0)
             {
                 OwnedRoxieString fname(helper.getFileName());
                 throw MakeStringException(0, "Invalid name %s in user metadata for index %s (names beginning with underscore are reserved)", name.str(), fname.get());

rtl/eclrtl/rtldynfield.cpp

@@ -903,6 +903,7 @@ extern ECLRTL_API bool dumpTypeInfo(MemoryBuffer &ret, const RtlTypeInfo *t)
     catch (IException *E)
     {
         EXCLOG(E);
+        E->Release();
         return false;
     }
 }

system/jlib/jstats.cpp

@@ -1756,25 +1756,14 @@ public:
 
     CStatisticCollection * ensureSubScope(const StatsScopeId & search, bool hasChildren)
     {
-        //MORE: Implement hasChildren
-        return resolveSubScope(search, true, false);
-    }
+        //Once the CStatisicCollection is created it should not be replaced - so that returned pointers remain valid.
+        CStatisticCollection * match = children.find(&search);
+        if (match)
+            return match;
 
-    CStatisticCollection * resolveSubScope(const StatsScopeId & search, bool create, bool replace)
-    {
-        if (!replace)
-        {
-            CStatisticCollection * match = children.find(&search);
-            if (match)
-                return LINK(match);
-        }
-        if (create)
-        {
-            CStatisticCollection * ret = new CStatisticCollection(this, search);
-            children.add(*ret);
-            return LINK(ret);
-        }
-        return NULL;
+        CStatisticCollection * ret = new CStatisticCollection(this, search);
+        children.add(*ret);
+        return ret;
     }
 
     virtual void serialize(MemoryBuffer & out) const
@@ -1985,7 +1974,6 @@ public:
     }
     virtual void endScope() override
     {
-        scopes.tos().Release();
         scopes.pop();
     }
     virtual void addStatistic(StatisticKind kind, unsigned __int64 value) override