Merge pull request #10401 from richardkchapman/collator-spinlock

HPCC-18243 Reduce scope of collatorsLock spinlock

Reviewed-by: Gavin Halliday <ghalliday@hpccsystems.com>

roxie/udplib/udptrr.cpp

@@ -819,7 +819,7 @@ class CReceiveManager : implements IReceiveManager, public CInterface
 
     Linked<IMessageCollator> defaultMessageCollator;
     uid_map         collators; // MORE - more sensible to use a jlib mapping I would have thought
-    SpinLock collatorsLock; // protects access to collators map and defaultMessageCollator
+    SpinLock collatorsLock; // protects access to collators map and defaultMessageCollator (note that defaultMessageCollator is not just set at startup)
 
 public:
     IMPLEMENT_IINTERFACE;
@@ -869,8 +869,10 @@ public:
     {
         ruid_t ruid = msgColl->queryRUID();
         if (udpTraceLevel >= 2) DBGLOG("UdpReceiver: detach %p %u", msgColl, ruid);
-        SpinBlock b(collatorsLock);
-        collators.erase(ruid); 
+        {
+            SpinBlock b(collatorsLock);
+            collators.erase(ruid);
+        }
         msgColl->Release();
     }
 
@@ -901,6 +903,7 @@ public:
                 pktHdr->ruid, pktHdr->msgId, pktHdr->msgSeq, pktHdr->pktSeq, pktHdr->length, pktHdr->nodeIndex);
 
         Linked <IMessageCollator> msgColl;
+        bool isDefault = false;
         {
             SpinBlock b(collatorsLock);
             try
@@ -908,9 +911,8 @@ public:
                 msgColl.set(collators[pktHdr->ruid]);
                 if (!msgColl)
                 {
-                    if (udpTraceLevel)
-                        DBGLOG("UdpReceiver: CPacketCollator NO msg collator found - using default - ruid=" RUIDF " id=0x%.8X mseq=%u pkseq=0x%.8X node=%u", pktHdr->ruid, pktHdr->msgId, pktHdr->msgSeq, pktHdr->pktSeq, pktHdr->nodeIndex);
                     msgColl.set(defaultMessageCollator); // MORE - if we get a header, we can send an abort.
+                    isDefault = true;
                 }
             }
             catch (IException *E)
@@ -927,6 +929,8 @@ public:
         }
         if (msgColl) 
         {
+            if (udpTraceLevel && isDefault)
+                DBGLOG("UdpReceiver: CPacketCollator NO msg collator found - using default - ruid=" RUIDF " id=0x%.8X mseq=%u pkseq=0x%.8X node=%u", pktHdr->ruid, pktHdr->msgId, pktHdr->msgSeq, pktHdr->pktSeq, pktHdr->nodeIndex);
             if (msgColl->add_package(dataBuff)) 
             {
                 dataBuff = 0;
@@ -949,9 +953,12 @@ public:
     virtual IMessageCollator *createMessageCollator(IRowManager *rowManager, ruid_t ruid)
     {
         IMessageCollator *msgColl = createCMessageCollator(rowManager, ruid);
-        if (udpTraceLevel >= 2) DBGLOG("UdpReceiver: createMessageCollator %p %u", msgColl, ruid);
-        SpinBlock b(collatorsLock);
-        collators[ruid] = msgColl;
+        if (udpTraceLevel >= 2)
+            DBGLOG("UdpReceiver: createMessageCollator %p %u", msgColl, ruid);
+        {
+            SpinBlock b(collatorsLock);
+            collators[ruid] = msgColl;
+        }
         msgColl->Link();
         return msgColl;
     }