HPCC-14849 Prevent BCD from deadlocking by using thread local storage

Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>

common/fileview2/fvresultset.cpp

@@ -1111,14 +1111,12 @@ xdouble CResultSetCursor::getDouble(int columnIndex)
             return (xdouble)rtlGetPackedUnsigned(cur);
     case type_decimal:
         {
-            DecLock();
+            BcdCriticalBlock bcdBlock;
             if (type.isSigned())
                 DecPushDecimal(cur, type.getSize(), type.getPrecision());
             else
                 DecPushUDecimal(cur, type.getSize(), type.getPrecision());
-            xdouble ret = DecPopReal();
-            DecUnlock();
-            return ret;
+            return DecPopReal();
         }
     case type_real:
         if (size == 4)
@@ -1188,14 +1186,12 @@ __int64 CResultSetCursor::getInt(int columnIndex)
             return rtlGetPackedUnsigned(cur);
     case type_decimal:
         {
-            DecLock();
+            BcdCriticalBlock bcdBlock;
             if (type.isSigned())
                 DecPushDecimal(cur, type.getSize(), type.getPrecision());
             else
                 DecPushUDecimal(cur, type.getSize(), type.getPrecision());
-            __int64 ret = DecPopInt64();
-            DecUnlock();
-            return ret;
+            return DecPopInt64();
         }
     case type_real:
         if (size == 4)
@@ -1346,13 +1342,12 @@ IStringVal & CResultSetCursor::getString(IStringVal & ret, int columnIndex)
         }
     case type_decimal:
         {
-            DecLock();
+            BcdCriticalBlock bcdBlock;
             if (type.isSigned())
                 DecPushDecimal(cur, type.getSize(), type.getPrecision());
             else
                 DecPushUDecimal(cur, type.getSize(), type.getPrecision());
             DecPopStringX(resultLen, resultStr);
-            DecUnlock();
             ret.setLen(resultStr, resultLen);
             return ret;
         }
@@ -1466,13 +1461,12 @@ IStringVal & CResultSetCursor::getDisplayText(IStringVal &ret, int columnIndex)
         }
     case type_decimal:
         {
-            DecLock();
+            BcdCriticalBlock bcdBlock;
             if (type.isSigned())
                 DecPushDecimal(cur, type.getSize(), type.getPrecision());
             else
                 DecPushUDecimal(cur, type.getSize(), type.getPrecision());
             DecPopStringX(resultLen, resultStr);
-            DecUnlock();
             ret.setLen(resultStr, resultLen);
             return ret;
         }
@@ -2473,13 +2467,13 @@ void CColumnFilter::addValue(unsigned sizeText, const char * text)
     case type_decimal:
         {
             void * target = next->allocate(size);
-            DecLock();
+
+            BcdCriticalBlock bcdBlock;
             rtlDecPushUtf8(lenText, text);
             if (type->isSigned())
                 DecPopDecimal(target, size, type->getPrecision());
             else
                 DecPopUDecimal(target, size, type->getPrecision());
-            DecUnlock();
             break;
         }
     case type_real:

common/thorhelper/thorxmlwrite.cpp

@@ -752,7 +752,8 @@ inline void outputEncodedXmlDecimal(const void *field, unsigned size, unsigned p
     char dec[50];
     if (fieldname)
         out.append('<').append(fieldname).append(" xsi:type=\"xsd:decimal\">");
-    DecLock();
+
+    BcdCriticalBlock bcdBlock;
     if (DecValid(true, size*2-1, field))
     {
         DecPushDecimal(field, size, precision);
@@ -763,7 +764,7 @@ inline void outputEncodedXmlDecimal(const void *field, unsigned size, unsigned p
     }
     else
         out.append("####");
-    DecUnlock();
+
     if (fieldname)
         out.append("</").append(fieldname).append('>');
 }
@@ -773,7 +774,8 @@ inline void outputEncodedXmlUDecimal(const void *field, unsigned size, unsigned
     char dec[50];
     if (fieldname)
         out.append('<').append(fieldname).append(" xsi:type=\"xsd:decimal\">");
-    DecLock();
+
+    BcdCriticalBlock bcdBlock;
     if (DecValid(false, size*2, field))
     {
         DecPushUDecimal(field, size, precision);
@@ -784,7 +786,7 @@ inline void outputEncodedXmlUDecimal(const void *field, unsigned size, unsigned
     }
     else
         out.append("####");
-    DecUnlock();
+
     if (fieldname)
         out.append("</").append(fieldname).append('>');
 }

ecl/hqlcpp/hqlcppsys.ecl

@@ -356,8 +356,6 @@ const char * cppSystemText[]  = {
     "   DecSwap() : eclrtl,library='eclrtl',entrypoint='DecSwap';",
     "   DecUint4Power(unsigned4 pow) :  eclrtl,library='eclrtl',entrypoint='DecUint4Power';",
     "   string DecPopStringX() :    eclrtl,library='eclrtl',entrypoint='DecPopStringX';",
-    "   DecLock() : eclrtl,library='eclrtl',entrypoint='DecLock';",
-    "   DecUnlock() :   eclrtl,library='eclrtl',entrypoint='DecUnlock';",
     "   boolean DecValid(boolean isSigned, const data src) : eclrtl,pure,library='eclrtl',entrypoint='DecValid';",
     "   boolean DecValidTos() : eclrtl,pure,library='eclrtl',entrypoint='DecValidTos';",
 

rtl/eclrtl/rtlbcd.cpp

@@ -22,20 +22,21 @@
 #include "jmutex.hpp"
 #include "jexcept.hpp"
 
-static CriticalSection bcdCriticalSection;
-static Decimal stack[32];
-static unsigned curStack;
+static thread_local Decimal stack[32];
+static thread_local unsigned curStack;
 
 //---------------------------------------------------------------------------------------------------------------------
 
+//These functions are retained to that old work units will load, and then report a version mismatch, rather than a
+//confusing unresolved symbol error.
 void DecLock()
 {
-    bcdCriticalSection.enter();
+    throwUnexpected();
 }
 
 void DecUnlock()
 {
-    bcdCriticalSection.leave();
+    throwUnexpected();
 }
 
 unsigned DecMarkStack()

rtl/eclrtl/rtlbcd.hpp

@@ -66,8 +66,6 @@ ECLRTL_API void  DecTruncate( void );       // truncate value on top of decimal
 ECLRTL_API void  DecTruncateAt(unsigned places);       // truncate value on top of decimal stack
 ECLRTL_API void  DecUlongPower(unsigned long pow); // calculates top of stack to the power of unsigned long and replaces with result
 
-ECLRTL_API void  DecLock();
-ECLRTL_API void  DecUnlock();
 ECLRTL_API bool  DecValid(bool isSigned, unsigned digits, const void * data);
 ECLRTL_API bool  DecValidTos();
 ECLRTL_API bool  Dec2Bool(size32_t bytes, const void * data);
@@ -87,11 +85,12 @@ ECLRTL_API void  DecUnlock();
 ECLRTL_API unsigned DecMarkStack();
 ECLRTL_API void DecReleaseStack(unsigned mark);
 
+//No longer a critical section (since stack is thread_local), but prevents problems with exceptions.
 class ECLRTL_API BcdCriticalBlock
 {
 public:
-    inline BcdCriticalBlock()       { DecLock(); mark = DecMarkStack(); }
-    inline ~BcdCriticalBlock()      { DecReleaseStack(mark); DecUnlock(); }
+    inline BcdCriticalBlock()       { mark = DecMarkStack(); }
+    inline ~BcdCriticalBlock()      { DecReleaseStack(mark); }
 
 protected:
     unsigned mark;

rtl/eclrtl/rtlxml.cpp

@@ -98,7 +98,8 @@ void outputXmlDecimal(const void *field, unsigned size, unsigned precision, cons
     char dec[50];
     if (fieldname && *fieldname)
         out.append('<').append(fieldname).append('>');
-    DecLock();
+
+    BcdCriticalBlock bcdBlock;
     if (DecValid(true, size*2-1, field))
     {
         DecPushDecimal(field, size, precision);
@@ -109,7 +110,7 @@ void outputXmlDecimal(const void *field, unsigned size, unsigned precision, cons
     }
     else
         out.append("####");
-    DecUnlock();
+
     if (fieldname && *fieldname)
         out.append("</").append(fieldname).append('>');
 }
@@ -119,7 +120,8 @@ void outputXmlUDecimal(const void *field, unsigned size, unsigned precision, con
     char dec[50];
     if (fieldname && *fieldname)
         out.append('<').append(fieldname).append('>');
-    DecLock();
+
+    BcdCriticalBlock bcdBlock;
     if (DecValid(false, size*2, field))
     {
         DecPushUDecimal(field, size, precision);
@@ -130,7 +132,7 @@ void outputXmlUDecimal(const void *field, unsigned size, unsigned precision, con
     }
     else
         out.append("####");
-    DecUnlock();
+
     if (fieldname && *fieldname)
         out.append("</").append(fieldname).append('>');
 }
@@ -253,7 +255,8 @@ void outputJsonDecimal(const void *field, unsigned size, unsigned precision, con
 {
     char dec[50];
     appendJSONNameOrDelimit(out, fieldname);
-    DecLock();
+
+    BcdCriticalBlock bcdBlock;
     if (DecValid(true, size*2-1, field))
     {
         DecPushDecimal(field, size, precision);
@@ -262,14 +265,14 @@ void outputJsonDecimal(const void *field, unsigned size, unsigned precision, con
         while(isspace(*finger)) finger++;
         out.append(finger);
     }
-    DecUnlock();
 }
 
 void outputJsonUDecimal(const void *field, unsigned size, unsigned precision, const char *fieldname, StringBuffer &out)
 {
     char dec[50];
     appendJSONNameOrDelimit(out, fieldname);
-    DecLock();
+
+    BcdCriticalBlock bcdBlock;
     if (DecValid(false, size*2, field))
     {
         DecPushUDecimal(field, size, precision);
@@ -278,5 +281,4 @@ void outputJsonUDecimal(const void *field, unsigned size, unsigned precision, co
         while(isspace(*finger)) finger++;
         out.append(finger);
     }
-    DecUnlock();
 }

testing/regress/ecl/bcd2.ecl

@@ -30,6 +30,10 @@ rec := { decimal17_4 sumx, unsigned countx };
 ds := dataset([{20,3},{10,2},{10.0001,2}], rec);
 output(nofold(ds), { sumx, countx, decimal20_10 average := sumx/countx, sumx between 10 and 10.00009, sumx between 10D and 10.00009D });
 
+ds1 := dataset(10000, TRANSFORM(rec, SELF.sumx := COUNTER * COUNTER; SELF.countx := COUNTER));
+ds2 := dataset(10000, TRANSFORM(rec, SELF.sumx := COUNTER * 2 * COUNTER; SELF.countx := COUNTER/3));
+ds3 := ds1 + ds2; // unoredered;
+output(SUM(nofold(ds3), sumx));
 
 decimal17_4 value1 := 1.6667;
 decimal17_4 value2 := 1.6667 : stored('value2');

testing/regress/ecl/key/bcd2.xml

@@ -10,7 +10,7 @@
  <Row><sumx>10.0001</sumx><countx>2</countx><average>5.00005</average><_unnamed_4>false</_unnamed_4><_unnamed_5>false</_unnamed_5></Row>
 </Dataset>
 <Dataset name='Result 4'>
- <Row><Result_4>2</Result_4></Row>
+ <Row><Result_4>1000150005000</Result_4></Row>
 </Dataset>
 <Dataset name='Result 5'>
  <Row><Result_5>2</Result_5></Row>
@@ -21,3 +21,6 @@
 <Dataset name='Result 7'>
  <Row><Result_7>2</Result_7></Row>
 </Dataset>
+<Dataset name='Result 8'>
+ <Row><Result_8>2</Result_8></Row>
+</Dataset>