From 156c0f4a6891d73d01b4012f3c6bacd1a642aa86 Mon Sep 17 00:00:00 2001 From: David Hermsmeier Date: Thu, 14 Nov 2019 17:10:09 -0500 Subject: [PATCH] bb:Altered the fix for the getHandle race condition above --- bb/src/BBTagInfoMap.cc | 8 +++++++- bb/src/TagInfo.cc | 24 +++++++++++++++++------- bb/src/TagInfo.h | 5 ++++- bb/src/bbserver.cc | 9 --------- 4 files changed, 28 insertions(+), 18 deletions(-) diff --git a/bb/src/BBTagInfoMap.cc b/bb/src/BBTagInfoMap.cc index 2c78bd0c9..ab4807aa1 100644 --- 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 index f5071de12..24fab7f83 100644 --- 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 index e6bf17f59..5ecf96a3d 100644 --- 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 index 572646db2..448edc846 100644 --- 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);