Use raii lock in DownloadBucketsWork

[SirTyson]

Description

Resolves #4674

Note that we are still using lock/unlock at DownloadBucketsWork.cpp:97, given that the type definition is incredibly verbose to forward declare, and it's unlikely std::map::emplace will throw. Even if it does, it doesn't matter that we hold the lock since core and the DownloadBucketsWork will crash anyway.

Checklist

• Reviewed the contributing document
• Rebased on top of master (no merge commits)
• Ran clang-format v8.0.0 (via make format or the Visual Studio extension)
• Compiles
• Ran all tests
• If change impacts performance, include supporting evidence per the performance document

[anupsdf]

lgtm

[greg7mdp]

Note that we are still using lock/unlock at DownloadBucketsWork.cpp:97, given that the type definition is incredibly verbose to forward declare

Not sure what C++ version core is using, but you shouldn't have to provide the type to std::lock_guard.

[SirTyson]

Note that we are still using lock/unlock at DownloadBucketsWork.cpp:97, given that the type definition is incredibly verbose to forward declare

Not sure what C++ version core is using, but you shouldn't have to provide the type to std::lock_guard.

I'm referring to the type returned by emplace, which must be assigned in the scope of the lock_guard but live outside the lock_guard scope.

[anupsdf]

Not sure what C++ version core is using

stellar-core is on C++17, https://github.com/stellar/stellar-core/blob/v22.2.0/configure.ac#L229

[greg7mdp]

I'm referring to the type returned by emplace, which must be assigned in the scope of the lock_guard but live outside the lock_guard scope.

Got it. You could use a lambda:

    auto [indexIter, inserted] = [&]() {
       std::lock_guard g(mIndexMapMutex);
       return mIndexMap.emplace(currId, nullptr);
    }();