Merge pull request #19235 from jakesmith/HPCC-32789-newsplitter-deadlock

HPCC-32789 Fix potential race deadlock in Thor splitter Reviewed-by: Gavin Halliday <[email protected]> Merged-by: Gavin Halliday <[email protected]>
hpcc-systems · Oct 25, 2024 · dd68be7 · dd68be7
2 parents 7b3f72c + 35a34fd
commit dd68be7
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 4 deletions.
diff --git a/thorlcr/thorutil/thbuf.cpp b/thorlcr/thorutil/thbuf.cpp
@@ -1272,6 +1272,10 @@ class CRowSet : public CSimpleInterface, implements IInterface
     {
         return rows.get(r);
     }
+    inline bool isFull() const
+    {
+        return rows.isFull();
+    }
 };
 
 class Chunk : public CInterface
@@ -1463,8 +1467,7 @@ class CSharedWriteAheadBase : public CSimpleInterface, implements ISharedSmartBu
                 }
             }
             rowsRead++;
-            const void *retrow = rowSet->getRow(row++);
-            return retrow;
+            return rowSet->getRow(row++);
         }
         virtual void stop()
         {
@@ -1693,7 +1696,8 @@ class CSharedWriteAheadBase : public CSimpleInterface, implements ISharedSmartBu
         unsigned len=rowSize(row);
         CriticalBlock b(crit);
         bool paged = false;
-        if (totalOutChunkSize >= minChunkSize) // chunks required to be at least minChunkSize
+        // NB: The isFull condition ensures that we never expand inMemRows, which would cause a race with readers reading same set
+        if (totalOutChunkSize >= minChunkSize || inMemRows->isFull()) // chunks required to be at least minChunkSize, or if hits max capacity
         {
             unsigned reader=anyReaderBehind();
             if (NotFound != reader)

diff --git a/thorlcr/thorutil/thmem.hpp b/thorlcr/thorutil/thmem.hpp
@@ -327,7 +327,8 @@ class graph_decl CThorExpandingRowArray : public CSimpleInterface
             if (!resize(numRows+1))
                 return false;
         }
-        rows[numRows++] = row;
+        rows[numRows] = row;
+        numRows++;
         return true;
     }
     bool binaryInsert(const void *row, ICompare &compare, bool dropLast=false); // NB: takes ownership on success
@@ -356,6 +357,7 @@ class graph_decl CThorExpandingRowArray : public CSimpleInterface
     }
     inline rowidx_t ordinality() const { return numRows; }
     inline rowidx_t queryMaxRows() const { return maxRows; }
+    inline bool isFull() const { return numRows >= maxRows; }
 
     inline const void **getRowArray() { return rows; }
     void swap(CThorExpandingRowArray &src);