Merge pull request #10410 from ghalliday/issue18250

HPCC-18250 Move gatherStats() from destructor to avoid accessing invalid memory Reviewed-By: Richard Chapman <rchapman@hpccsystems.com>
hpcc-systems · Sep 15, 2017 · cea1393 · cea1393
2 parents a6fb5a3 + 7d8161c
commit cea1393
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 34 deletions.
diff --git a/roxie/ccd/ccdcontext.cpp b/roxie/ccd/ccdcontext.cpp
@@ -1481,6 +1481,16 @@ class CRoxieContextBase : implements IRoxieSlaveContext, implements ICodeContext
 
     void cleanupGraphs()
     {
+        if (graph)
+            graph->updateFactoryStatistics();
+
+        SuperHashIteratorOf<decltype(childGraphs)::ELEMENT> iter(childGraphs);
+        ForEach(iter)
+        {
+            IActivityGraph * curChildGraph = static_cast<IActivityGraph *>(iter.query().getValue());
+            curChildGraph->updateFactoryStatistics();
+        }
+
         graph.clear();
         childGraphs.kill();
     }

diff --git a/roxie/ccd/ccdquery.hpp b/roxie/ccd/ccdquery.hpp
@@ -67,6 +67,7 @@ interface IActivityGraph : extends IInterface
     virtual IRoxieServerChildGraph * queryLoopGraph() = 0;
     virtual IRoxieServerChildGraph * createGraphLoopInstance(IRoxieSlaveContext *ctx, unsigned loopCounter, unsigned parentExtractSize, const byte * parentExtract, const IRoxieContextLogger &logctx) = 0;
     virtual const char *queryName() const = 0;
+    virtual void updateFactoryStatistics() const = 0;
 };
 
 interface IRoxiePackage;

diff --git a/roxie/ccd/ccdserver.cpp b/roxie/ccd/ccdserver.cpp
@@ -1011,7 +1011,7 @@ class CRoxieServerActivity : implements CInterfaceOf<IRoxieServerActivity>, impl
         }
     }
 
-    virtual void updateFactoryStatistics() override
+    virtual void updateFactoryStatistics() const override
     {
         CRuntimeStatisticCollection mergedStats(stats.queryMapping());
         gatherStats(mergedStats);
@@ -1028,6 +1028,9 @@ class CRoxieServerActivity : implements CInterfaceOf<IRoxieServerActivity>, impl
                 ctx->noteProcessed(factory->querySubgraphId(), activityId, 0, processed, 0);
             ctx->mergeActivityStats(mergedStats, factory->querySubgraphId(), activityId);
         }
+
+        ForEachItemIn(i, childGraphs)
+            childGraphs.item(i).updateFactoryStatistics();
     }
 
     virtual const IRoxieContextLogger &queryLogCtx()const
@@ -15776,6 +15779,10 @@ class CRoxieServerParallelGraphLoopActivity : public CRoxieServerGraphLoopActivi
         resultInput = NULL;
         resultStream = NULL;
         resultJunction.clear();
+
+        ForEachItemIn(i, iterationGraphs)
+            iterationGraphs.item(i).updateFactoryStatistics();
+
         outputs.kill();
         iterationGraphs.kill(); // must be done after all activities killed
         if (probeManager)
@@ -16182,6 +16189,13 @@ class CRoxieServerLibraryCallActivity : public CRoxieServerActivity
         outputUsed[idx] = true;
         return &outputAdaptors[idx];
     }
+
+    virtual void updateFactoryStatistics() const override
+    {
+        if (libraryGraph)
+            libraryGraph->updateFactoryStatistics();
+        CRoxieServerActivity::updateFactoryStatistics();
+    }
 };
 
 
@@ -27116,8 +27130,6 @@ class CActivityGraph : implements IActivityGraph, implements IThorChildGraph, im
 
     ~CActivityGraph()
     {
-        ForEachItemIn(i, activities)
-            activities.item(i).updateFactoryStatistics();
         if (probeManager)
             probeManager->deleteGraph((IArrayOf<IActivityBase>*)&activities, (IArrayOf<IInputBase>*)&probes);
     }
@@ -27342,6 +27354,12 @@ class CActivityGraph : implements IActivityGraph, implements IThorChildGraph, im
         return results.getClear();
     }
 
+    virtual void updateFactoryStatistics() const override
+    {
+        ForEachItemIn(i, activities)
+            activities.item(i).updateFactoryStatistics();
+    }
+
     //interface IRoxieServerChildGraph
     virtual void beforeExecute()
     {
@@ -27494,24 +27512,28 @@ class CProxyActivityGraph : implements IActivityGraph, implements IThorChildGrap
         : ctx(_ctx), graphName(_graphName), id(_id), graphDefinition(_graphDefinition), logctx(_logctx), numParallel(_numParallel)
     {
     }
-    virtual void abort() { throwUnexpected(); }
-    virtual void reset() { throwUnexpected(); }
-    virtual void execute() { throwUnexpected(); }
-    virtual void getProbeResponse(IPropertyTree *query) { throwUnexpected(); }
-    virtual void onCreate(IHThorArg *_colocalArg)
+    virtual void abort() override { throwUnexpected(); }
+    virtual void reset() override { throwUnexpected(); }
+    virtual void execute() override { throwUnexpected(); }
+    virtual void getProbeResponse(IPropertyTree *query) override { throwUnexpected(); }
+    virtual void onCreate(IHThorArg *_colocalArg) override
     {
         colocalArg = _colocalArg;
     }
-    virtual void noteException(IException *E) { throwUnexpected(); }
-    virtual void checkAbort() { throwUnexpected(); }
-    virtual IThorChildGraph * queryChildGraph() { return this; }
-    virtual IEclGraphResults * queryLocalGraph() { throwUnexpected(); }
-    virtual IRoxieServerChildGraph * queryLoopGraph() { throwUnexpected(); }
-    virtual IRoxieServerChildGraph * createGraphLoopInstance(IRoxieSlaveContext *ctx, unsigned loopCounter, unsigned parentExtractSize, const byte * parentExtract, const IRoxieContextLogger &logctx) { throwUnexpected(); }
-    virtual const char *queryName() const { throwUnexpected(); }
-    virtual IRoxieServerActivity *queryActivity(unsigned _activityId) { return nullptr; } // MORE - may need something here!?
-
-    virtual IEclGraphResults * evaluate(unsigned parentExtractSize, const byte * parentExtract)
+    virtual void noteException(IException *E) override { throwUnexpected(); }
+    virtual void checkAbort() override { throwUnexpected(); }
+    virtual IThorChildGraph * queryChildGraph() override { return this; }
+    virtual IEclGraphResults * queryLocalGraph() override { throwUnexpected(); }
+    virtual IRoxieServerChildGraph * queryLoopGraph() override { throwUnexpected(); }
+    virtual IRoxieServerChildGraph * createGraphLoopInstance(IRoxieSlaveContext *ctx, unsigned loopCounter, unsigned parentExtractSize, const byte * parentExtract, const IRoxieContextLogger &logctx) override { throwUnexpected(); }
+    virtual const char *queryName() const override { throwUnexpected(); }
+    virtual void updateFactoryStatistics() const override
+    {
+        CriticalBlock b(graphCrit);
+        ForEachItemIn(i, stack)
+            stack.item(i).updateFactoryStatistics();
+    }
+    virtual IEclGraphResults * evaluate(unsigned parentExtractSize, const byte * parentExtract) override
     {
         Owned<CActivityGraph> realGraph;
         {
@@ -27541,7 +27563,7 @@ class CProxyActivityGraph : implements IActivityGraph, implements IThorChildGrap
     const IRoxieContextLogger &logctx;
     unsigned numParallel;
     IHThorArg *colocalArg = nullptr;
-    CriticalSection graphCrit;
+    mutable CriticalSection graphCrit;
     CIArrayOf<CActivityGraph> stack;
 };
 
@@ -27722,24 +27744,23 @@ class CDelayedActivityGraph : implements IActivityGraph, public CInterface
         colocalParent = NULL;
     }
 
-    virtual const char *queryName() const { return graphName.get(); }
-    virtual void abort() { throwUnexpected(); }
-    virtual void reset() { }
-    virtual void execute() { throwUnexpected(); }
-    virtual void getProbeResponse(IPropertyTree *query) { throwUnexpected(); }
-    virtual void noteException(IException *E) { throwUnexpected(); }
-    virtual void checkAbort() { throwUnexpected(); }
-    virtual IThorChildGraph * queryChildGraph() { throwUnexpected(); }
-    virtual IEclGraphResults * queryLocalGraph() { throwUnexpected(); }
-    virtual IRoxieServerChildGraph * queryLoopGraph() { throwUnexpected(); }
-    virtual IRoxieServerActivity *queryActivity(unsigned _activityId) { return nullptr; } // MORE - may need something here!?
-
-    virtual void onCreate(IHThorArg *_colocalParent)
+    virtual const char *queryName() const override { return graphName.get(); }
+    virtual void abort() override { throwUnexpected(); }
+    virtual void reset() override { }
+    virtual void execute() override { throwUnexpected(); }
+    virtual void getProbeResponse(IPropertyTree *query) override { throwUnexpected(); }
+    virtual void noteException(IException *E) override { throwUnexpected(); }
+    virtual void checkAbort() override { throwUnexpected(); }
+    virtual IThorChildGraph * queryChildGraph() override { throwUnexpected(); }
+    virtual IEclGraphResults * queryLocalGraph() override { throwUnexpected(); }
+    virtual IRoxieServerChildGraph * queryLoopGraph() override { throwUnexpected(); }
+    virtual void updateFactoryStatistics() const override { }
+    virtual void onCreate(IHThorArg *_colocalParent) override
     { 
         colocalParent = _colocalParent;
     }
 
-    virtual IRoxieServerChildGraph * createGraphLoopInstance(IRoxieSlaveContext *ctx, unsigned loopCounter, unsigned parentExtractSize, const byte * parentExtract, const IRoxieContextLogger &logctx)
+    virtual IRoxieServerChildGraph * createGraphLoopInstance(IRoxieSlaveContext *ctx, unsigned loopCounter, unsigned parentExtractSize, const byte * parentExtract, const IRoxieContextLogger &logctx) override
     {
         Owned<CIterationActivityGraph> ret = new CIterationActivityGraph(graphName, id, graphDefinition, probeManager, loopCounter, ctx, colocalParent, parentExtractSize, parentExtract, logctx);
         ret->createIterationGraph(ctx);

diff --git a/roxie/ccd/ccdserver.hpp b/roxie/ccd/ccdserver.hpp
@@ -191,7 +191,7 @@ interface IRoxieServerActivity : extends IActivityBase
     virtual void mergeStats(MemoryBuffer &stats) = 0;
     virtual ISectionTimer * registerTimer(unsigned activityId, const char * name) = 0;
     virtual IEngineRowAllocator * createRowAllocator(IOutputMetaData * metadata) = 0;
-    virtual void updateFactoryStatistics() = 0;
+    virtual void updateFactoryStatistics() const = 0;
 };
 
 interface IRoxieServerActivityFactory : extends IActivityFactory
@@ -267,6 +267,7 @@ interface IRoxieServerChildGraph : public IInterface
     virtual CGraphIterationInfo * selectGraphLoopOutput() = 0;
     virtual void gatherIterationUsage(IRoxieServerLoopResultProcessor & processor) = 0;
     virtual void associateIterationOutputs(IRoxieServerLoopResultProcessor & processor) = 0;
+    virtual void updateFactoryStatistics() const = 0;
 };
 
 interface IQueryFactory;

diff --git a/system/jlib/jhash.hpp b/system/jlib/jhash.hpp
@@ -298,6 +298,8 @@ class MappingXToIInterface : public MappingBetween<KEY, KEYINIT, IInterfacePtr,I
 template <class KEY, class KEYINIT>
 class MapXToIInterface  : public MapBetween<KEY, KEYINIT, IInterfacePtr,IInterfacePtr,MappingXToIInterface<KEY, KEYINIT> >
 {
+public:
+    using ELEMENT = MappingXToIInterface<KEY, KEYINIT>;
 };
 
 template <class KEY, class KEYINIT, class C>