Merge pull request #822 from IBM/dev-dlherms2

bb:Altered the fix for the getHandle race condition above
IBM · Nov 15, 2019 · fb2faea · fb2faea
2 parents 7ef15cf + 156c0f4
commit fb2faea
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 18 deletions.
diff --git a/bb/src/BBTagInfoMap.cc b/bb/src/BBTagInfoMap.cc
@@ -57,9 +57,10 @@ int BBTagInfoMap::addTagInfo(const LVKey* pLVKey, const BBJob pJob, const BBTagI
 
     LOG(bb,debug) << "BBTagInfoMap::addTagInfo(): LVKey " << *pLVKey << ", job (" << pJob.getJobId() << "," << pJob.getJobStepId() << "), tagid " << pTagId.getTag() << ", generated handle " << pGeneratedHandle;
 
-    // It is possible to enter this section of code without the transfer queue locked.
+    // It is possible to enter this section of code without the local metadata locked.
     // Inserting into a std::map is not thread safe, so we must acquire the lock around
     // the insert.
+    int l_TransferQueueWasUnlocked = unlockTransferQueueIfNeeded((LVKey*)0, "BBTagInfoMap::getTagInfo");
     int l_LocalMetadataWasLocked = lockLocalMetadataIfNeeded(pLVKey, "BBTagInfoMap::addTagInfo");
 
     tagInfoMap[pTagId] = *pTagInfo;
@@ -70,6 +71,11 @@ int BBTagInfoMap::addTagInfo(const LVKey* pLVKey, const BBJob pJob, const BBTagI
         unlockLocalMetadata(pLVKey, "BBTagInfoMap::addTagInfo");
     }
 
+    if (l_TransferQueueWasUnlocked)
+    {
+        lockTransferQueue((LVKey*)0, "BBTagInfoMap::getTagInfo");
+    }
+
     if (pGeneratedHandle) {
         rc = update_xbbServerAddData(pLVKey, pJob, pTagId.getTag(), pTagInfo);
     }

diff --git a/bb/src/TagInfo.cc b/bb/src/TagInfo.cc
@@ -36,10 +36,10 @@ int TagInfo::addTagHandle(const LVKey* pLVKey, const BBJob pJob, const uint64_t
 
     TagInfo* l_TagInfo = 0;
     HandleInfo* l_HandleInfo = 0;
+    int l_TransferQueueWasUnlocked = 0;
+    int l_LocalMetadataLocked = 0;
     int l_TagInfoLocked = 0;
 
-    int l_LocalMetadataUnlocked = unlockLocalMetadataIfNeeded(pLVKey, "addTagHandle");
-
     try
     {
         bfs::path l_JobStepPath(g_BBServer_Metadata_Path);
@@ -52,7 +52,12 @@ int TagInfo::addTagHandle(const LVKey* pLVKey, const BBJob pJob, const uint64_t
             LOG_ERROR_TEXT_RC_AND_BAIL(errorText, rc);
         }
 
+        l_TransferQueueWasUnlocked = unlockTransferQueueIfNeeded(pLVKey, "addTagHandle");
+        // This lock serializes amongst request/transfer threads on this bbServer...
+        l_LocalMetadataLocked = lockLocalMetadataIfNeeded(pLVKey, "addTagHandle");
+        // This lock serializes amongst bbServers...
         rc = TagInfo::lock(l_JobStepPath);
+
         if (!rc)
         {
             l_TagInfoLocked = 1;
@@ -150,6 +155,16 @@ int TagInfo::addTagHandle(const LVKey* pLVKey, const BBJob pJob, const uint64_t
         TagInfo::unlock();
     }
 
+    if (l_LocalMetadataLocked)
+    {
+        unlockLocalMetadata(pLVKey, "addTagHandle");
+    }
+
+    if (l_TransferQueueWasUnlocked)
+    {
+        lockTransferQueue(pLVKey, "addTagHandle");
+    }
+
     if (l_HandleInfo)
     {
         delete l_HandleInfo;
@@ -162,11 +177,6 @@ int TagInfo::addTagHandle(const LVKey* pLVKey, const BBJob pJob, const uint64_t
         l_TagInfo = 0;
     }
 
-    if (l_LocalMetadataUnlocked)
-    {
-        lockLocalMetadata(pLVKey, "addTagHandle");
-    }
-
     return rc;
 }
 

diff --git a/bb/src/TagInfo.h b/bb/src/TagInfo.h
@@ -54,7 +54,10 @@ const int MAXIMUM_TAGINFO_LOADTIME = 10;     // In seconds
  | External methods
  *******************************************************************************/
 extern void lockLocalMetadata(const LVKey* pLVKey, const char* pMethod);
-extern int unlockLocalMetadataIfNeeded(const LVKey* pLVKey, const char* pMethod);
+extern int lockLocalMetadataIfNeeded(const LVKey* pLVKey, const char* pMethod);
+extern void unlockLocalMetadata(const LVKey* pLVKey, const char* pMethod);
+extern void lockTransferQueue(const LVKey* pLVKey, const char* pMethod);
+extern int unlockTransferQueueIfNeeded(const LVKey* pLVKey, const char* pMethod);
 
 
 /*******************************************************************************

diff --git a/bb/src/bbserver.cc b/bb/src/bbserver.cc
@@ -700,7 +700,6 @@ void msgin_gettransferhandle(txp::Id id, const std::string& pConnectionName, txp
     verifyInitLockState();
 
     uint64_t l_Handle = UNDEFINED_HANDLE;
-    int l_LocalMetadataLocked = 0;
     LVKey l_LVKey;
     LVKey* l_LVKeyPtr = &l_LVKey;
     char lv_uuid_str[LENGTH_UUID_STR] = {'\0'};
@@ -746,8 +745,6 @@ void msgin_gettransferhandle(txp::Id id, const std::string& pConnectionName, txp
 
         switchIds(msg);
 
-        lockLocalMetadata((LVKey*)0, "msgin_gettransferhandle");
-        l_LocalMetadataLocked = 1;
         //  NOTE:  We set up to wait 2 minutes for the necessary LVKey to appear if we can't find
         //         it right away and the handle is not in the cross-bbServer metadata.
         //         This closes the window during activate server between the activation
@@ -844,12 +841,6 @@ void msgin_gettransferhandle(txp::Id id, const std::string& pConnectionName, txp
         LOG_ERROR_RC_WITH_EXCEPTION(__FILE__, __FUNCTION__, __LINE__, e, rc);
     }
 
-    if (l_LocalMetadataLocked)
-    {
-        l_LocalMetadataLocked = 0;
-        unlockLocalMetadata((LVKey*)0, "msgin_gettransferhandle");
-    }
-
     // Build the response message
     txp::Msg* response;
     msg->buildResponseMsg(response);