Browse Source

HPCC-14668 Fix destructors that can throw exceptions

Few good fixes available.

Signed-off-by: Richard Chapman <rchapman@hpccsystems.com>
Richard Chapman 9 years ago
parent
commit
f549a8e089

+ 1 - 1
dali/base/dasds.cpp

@@ -5236,7 +5236,7 @@ class CStoreHelper : public CInterface, implements IStoreHelper
                 MilliSleep(100);
             }
         }
-        ~CheckDeltaBlock()
+        ~CheckDeltaBlock() noexcept(false)
         {
             if (detachIPIFile)
             {

+ 17 - 9
system/jlib/jiface.hpp

@@ -44,20 +44,18 @@ void jlib_decl raiseAssertCore(const char *assertion, const char *file, unsigned
  #define assertex(p)
  #define assert(p)
 #endif
-
 #if defined(_DEBUG)||defined(_TESTING)
-#define verifyex(p) (likely(p) ? ((void) 0) : (( (void) raiseAssertException(#p, __FILE__, __LINE__))))
-#define verify(p) (likely(p) ? ((void) 0) : (( (void) raiseAssertCore(#p, __FILE__, __LINE__))))
+ #define verifyex(p) (likely(p) ? ((void) 0) : (( (void) raiseAssertException(#p, __FILE__, __LINE__))))
+ #define verify(p) (likely(p) ? ((void) 0) : (( (void) raiseAssertCore(#p, __FILE__, __LINE__))))
 #else
-#define verifyex(p) ((void) (p))
-#define verify(p) ((void) (p))
+ #define verifyex(p) ((void) (p))
+ #define verify(p) ((void) (p))
 #endif
 
-//Use for asserts that are highly unlikely to occur, and would likely to be reproduced in debug mode.
 #ifdef _DEBUG
-#define dbgassertex(x) assertex(x)
+ #define dbgassertex(x) assertex(x)   //Use for asserts that are highly unlikely to occur, and would likely to be reproduced in debug mode.
 #else
-#define dbgassertex(x)
+ #define dbgassertex(x)
 #endif
 
 #define DEAD_PSEUDO_COUNT               0x3fffffff
@@ -128,7 +126,17 @@ public:
             //to a a high mid-point positive number to avoid poss. of releasing again.
             if (this->xxcount.compare_exchange_strong(zero, DEAD_PSEUDO_COUNT, std::memory_order_acq_rel))
             {
-                const_cast<CInterfaceOf<INTERFACE> *>(this)->beforeDispose();
+                try
+                {
+                    const_cast<CInterfaceOf<INTERFACE> *>(this)->beforeDispose();
+                }
+                catch (...)
+                {
+#if _DEBUG
+                    assert(!"ERROR - Exception in beforeDispose - object will be leaked");
+#endif
+                    throw;
+                }
                 delete this;
                 return true;
             }

+ 19 - 5
system/jlib/jio.cpp

@@ -1140,13 +1140,27 @@ public:
 
     ~CBufferedIIOStream()
     {
-        try { flush(); }
-        catch (IException *)
+        delete [] buffer;
+    }
+
+    virtual void beforeDispose()
+    {
+        try
         {
-            delete [] buffer;
-            throw;
+            // NOTE - flush may throw an exception and thus cannot be done in the destructor.
+            flush();
+        }
+        catch (IException *E)
+        {
+            EXCLOG(E, "ERROR - Exception in CBufferedIIOStream::flush ignored");
+            E->Release();
+            assert(!"ERROR - Exception in CBufferedIIOStream::flush ignored");
+        }
+        catch (...)
+        {
+            DBGLOG("ERROR - Unknown exception in CBufferedIIOStream::flush ignored");
+            assert(!"ERROR - Unknown exception in CBufferedIIOStream::flush ignored");
         }
-        delete [] buffer;
     }
 
     virtual bool fillBuffer()

+ 3 - 1
thorlcr/activities/thactivityutil.cpp

@@ -675,8 +675,10 @@ public:
         if (globals->getPropBool("@replicateAsync", true))
             cancelReplicates(&activity, partDesc);
     }
-    ~CWriteHandler()
+    virtual void beforeDispose()
     {
+        // Can't throw in destructor...
+        // Note that if we do throw the CWriteHandler object is liable to be leaked...
         primaryio.clear(); // should close
         if (aborted && *aborted)
         {

+ 3 - 1
thorlcr/slave/slave.cpp

@@ -55,8 +55,10 @@ ProcessSlaveActivity::ProcessSlaveActivity(CGraphElementBase *container) : CSlav
     lastCycles = 0;
 }
 
-ProcessSlaveActivity::~ProcessSlaveActivity()
+void ProcessSlaveActivity::beforeDispose()
 {
+    // Note - we can't throw from the destructor, so do this in beforeDispose instead
+    // If the exception is thrown then we are liable to leak the object, but we are dying anyway...
     ActPrintLog("destroying ProcessSlaveActivity");
     ActPrintLog("ProcessSlaveActivity : joining process thread");
     // NB: The activity thread should have already stopped,

+ 1 - 1
thorlcr/slave/slave.ipp

@@ -45,7 +45,7 @@ public:
     IMPLEMENT_IINTERFACE_USING(CSlaveActivity);
 
     ProcessSlaveActivity(CGraphElementBase *container);
-    ~ProcessSlaveActivity();
+    virtual void beforeDispose();
 
     virtual void startProcess(bool async=true);
     virtual bool wait(unsigned timeout);