C++ 17 style critical sections

Summary:
C++ 17 added `std::scoped_lock`, which effectively supersedes `std::lock_guard`. See https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp21-use-stdlock-or-stdscoped_lock-to-acquire-multiple-mutexes for why it was added.

This diff replaces usages of `std::lock_guard` with `std::scoped_lock` and CTAD.

Changelog:
[Internal]

Reviewed By: mdvacca

Differential Revision: D48244308

fbshipit-source-id: d0a090d92c4c7276fdeea7fb1275900e5e0d3617

Author: NickGerleman

packages/react-native/Libraries/WebPerformance/PerformanceEntryReporter.cpp

@@ -66,7 +66,7 @@ void PerformanceEntryReporter::stopReporting() {
 }
 
 GetPendingEntriesResult PerformanceEntryReporter::popPendingEntries() {
-  std::lock_guard<std::mutex> lock(entriesMutex_);
+  std::scoped_lock lock(entriesMutex_);
   GetPendingEntriesResult res = {
       std::vector<RawPerformanceEntry>(), droppedEntryCount_};
   for (auto &buffer : buffers_) {
@@ -99,7 +99,7 @@ void PerformanceEntryReporter::logEntry(const RawPerformanceEntry &entry) {
     return;
   }
 
-  std::lock_guard<std::mutex> lock(entriesMutex_);
+  std::scoped_lock lock(entriesMutex_);
 
   auto &buffer = buffers_[entry.entryType];
 
@@ -367,7 +367,7 @@ EventTag PerformanceEntryReporter::onEventStart(const char *name) {
 
   auto timeStamp = getCurrentTimeStamp();
   {
-    std::lock_guard<std::mutex> lock(eventsInFlightMutex_);
+    std::scoped_lock lock(eventsInFlightMutex_);
     eventsInFlight_.emplace(std::make_pair(
         sCurrentEventTag_, EventEntry{reportedName, timeStamp, 0.0}));
   }
@@ -380,7 +380,7 @@ void PerformanceEntryReporter::onEventDispatch(EventTag tag) {
   }
   auto timeStamp = getCurrentTimeStamp();
   {
-    std::lock_guard<std::mutex> lock(eventsInFlightMutex_);
+    std::scoped_lock lock(eventsInFlightMutex_);
     auto it = eventsInFlight_.find(tag);
     if (it != eventsInFlight_.end()) {
       it->second.dispatchTime = timeStamp;
@@ -394,7 +394,7 @@ void PerformanceEntryReporter::onEventEnd(EventTag tag) {
   }
   auto timeStamp = getCurrentTimeStamp();
   {
-    std::lock_guard<std::mutex> lock(eventsInFlightMutex_);
+    std::scoped_lock lock(eventsInFlightMutex_);
     auto it = eventsInFlight_.find(tag);
     if (it == eventsInFlight_.end()) {
       return;

packages/react-native/ReactAndroid/src/main/jni/react/fabric/EventBeatManager.cpp

@@ -22,18 +22,18 @@ jni::local_ref<EventBeatManager::jhybriddata> EventBeatManager::initHybrid(
 
 void EventBeatManager::addObserver(
     EventBeatManagerObserver const &observer) const {
-  std::lock_guard<std::mutex> lock(mutex_);
+  std::scoped_lock lock(mutex_);
   observers_.insert(&observer);
 }
 
 void EventBeatManager::removeObserver(
     EventBeatManagerObserver const &observer) const {
-  std::lock_guard<std::mutex> lock(mutex_);
+  std::scoped_lock lock(mutex_);
   observers_.erase(&observer);
 }
 
 void EventBeatManager::tick() {
-  std::lock_guard<std::mutex> lock(mutex_);
+  std::scoped_lock lock(mutex_);
 
   for (auto observer : observers_) {
     observer->tick();

packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp

@@ -250,7 +250,7 @@ void FabricMountingManager::executeMount(
     const MountingTransaction &transaction) {
   SystraceSection section("FabricMountingManager::executeMount");
 
-  std::lock_guard<std::recursive_mutex> lock(commitMutex_);
+  std::scoped_lock lock(commitMutex_);
   auto finishTransactionStartTime = telemetryTimePointNow();
 
   auto env = jni::Environment::current();

packages/react-native/ReactAndroid/src/main/jni/react/jni/JMessageQueueThread.cpp

@@ -88,7 +88,7 @@ void JMessageQueueThread::runOnQueueSync(std::function<void()> &&runnable) {
     bool runnableComplete = false;
 
     runOnQueue([&]() mutable {
-      std::lock_guard<std::mutex> lock(signalMutex);
+      std::scoped_lock lock(signalMutex);
 
       runnable();
       runnableComplete = true;

packages/react-native/ReactCommon/cxxreact/Instance.cpp

@@ -59,7 +59,7 @@ void Instance::initializeBridge(
      */
     jsCallInvoker_->setNativeToJsBridgeAndFlushCalls(nativeToJsBridge_);
 
-    std::lock_guard<std::mutex> lock(m_syncMutex);
+    std::scoped_lock lock(m_syncMutex);
     m_syncReady = true;
     m_syncCV.notify_all();
   });
@@ -248,7 +248,7 @@ Instance::getDecoratedNativeMethodCallInvoker(
 
 void Instance::JSCallInvoker::setNativeToJsBridgeAndFlushCalls(
     std::weak_ptr<NativeToJsBridge> nativeToJsBridge) {
-  std::lock_guard<std::mutex> guard(m_mutex);
+  std::scoped_lock guard(m_mutex);
 
   m_shouldBuffer = false;
   m_nativeToJsBridge = nativeToJsBridge;
@@ -265,7 +265,7 @@ void Instance::JSCallInvoker::invokeSync(std::function<void()> &&work) {
 }
 
 void Instance::JSCallInvoker::invokeAsync(std::function<void()> &&work) {
-  std::lock_guard<std::mutex> guard(m_mutex);
+  std::scoped_lock guard(m_mutex);
 
   /**
    * Why is is necessary to queue up async work?

packages/react-native/ReactCommon/hermes/inspector/Inspector.cpp

@@ -117,7 +117,7 @@ Inspector::Inspector(
       std::make_shared<jsi::StringBuffer>(src), "__tickleJsHackUrl");
 
   {
-    std::lock_guard<std::mutex> lock(mutex_);
+    std::scoped_lock lock(mutex_);
 
     if (pauseOnFirstStatement) {
       awaitingDebuggerOnStart_ = true;
@@ -289,10 +289,11 @@ ScriptInfo Inspector::getScriptInfoFromTopCallFrame() {
 
 void Inspector::addCurrentScriptToLoadedScripts() {
   ScriptInfo info = getScriptInfoFromTopCallFrame();
+  auto fileId = info.fileId;
 
-  if (!loadedScripts_.count(info.fileId)) {
-    loadedScriptIdByName_[info.fileName] = info.fileId;
-    loadedScripts_[info.fileId] = LoadedScriptInfo{std::move(info), false};
+  if (!loadedScripts_.count(fileId)) {
+    loadedScriptIdByName_[info.fileName] = fileId;
+    loadedScripts_[fileId] = LoadedScriptInfo{std::move(info), false};
   }
 }
 
@@ -538,7 +539,7 @@ void Inspector::transition(std::unique_ptr<InspectorState> nextState) {
 
 void Inspector::disableOnExecutor(
     std::shared_ptr<folly::Promise<Unit>> promise) {
-  std::lock_guard<std::mutex> lock(mutex_);
+  std::scoped_lock lock(mutex_);
 
   debugger_.setIsDebuggerAttached(false);
 
@@ -547,7 +548,7 @@ void Inspector::disableOnExecutor(
 
 void Inspector::enableOnExecutor(
     std::shared_ptr<folly::Promise<Unit>> promise) {
-  std::lock_guard<std::mutex> lock(mutex_);
+  std::scoped_lock lock(mutex_);
 
   auto result = state_->enable();
 
@@ -575,7 +576,7 @@ void Inspector::executeIfEnabledOnExecutor(
     const std::string &description,
     folly::Function<void(const debugger::ProgramState &)> func,
     std::shared_ptr<folly::Promise<Unit>> promise) {
-  std::lock_guard<std::mutex> lock(mutex_);
+  std::scoped_lock lock(mutex_);
 
   if (!state_->isPaused() && !state_->isRunning()) {
     promise->setException(InvalidStateException(
@@ -601,7 +602,7 @@ void Inspector::setBreakpointOnExecutor(
     debugger::SourceLocation loc,
     std::optional<std::string> condition,
     std::shared_ptr<folly::Promise<debugger::BreakpointInfo>> promise) {
-  std::lock_guard<std::mutex> lock(mutex_);
+  std::scoped_lock lock(mutex_);
 
   bool pushed = state_->pushPendingFunc([this, loc, condition, promise] {
     debugger::BreakpointID id = debugger_.setBreakpoint(loc);
@@ -625,7 +626,7 @@ void Inspector::setBreakpointOnExecutor(
 void Inspector::removeBreakpointOnExecutor(
     debugger::BreakpointID breakpointId,
     std::shared_ptr<folly::Promise<folly::Unit>> promise) {
-  std::lock_guard<std::mutex> lock(mutex_);
+  std::scoped_lock lock(mutex_);
 
   bool pushed = state_->pushPendingFunc([this, breakpointId, promise] {
     debugger_.deleteBreakpoint(breakpointId);
@@ -640,7 +641,7 @@ void Inspector::removeBreakpointOnExecutor(
 void Inspector::logOnExecutor(
     ConsoleMessageInfo info,
     std::shared_ptr<folly::Promise<folly::Unit>> promise) {
-  std::lock_guard<std::mutex> lock(mutex_);
+  std::scoped_lock lock(mutex_);
 
   state_->pushPendingFunc([this, info = std::move(info)] {
     observer_.onMessageAdded(*this, info);
@@ -652,13 +653,13 @@ void Inspector::logOnExecutor(
 void Inspector::setPendingCommandOnExecutor(
     debugger::Command command,
     std::shared_ptr<folly::Promise<Unit>> promise) {
-  std::lock_guard<std::mutex> lock(mutex_);
+  std::scoped_lock lock(mutex_);
 
   state_->setPendingCommand(std::move(command), promise);
 }
 
 void Inspector::pauseOnExecutor(std::shared_ptr<folly::Promise<Unit>> promise) {
-  std::lock_guard<std::mutex> lock(mutex_);
+  std::scoped_lock lock(mutex_);
 
   bool canPause = state_->pause();
 
@@ -675,7 +676,7 @@ void Inspector::evaluateOnExecutor(
     std::shared_ptr<folly::Promise<debugger::EvalResult>> promise,
     folly::Function<void(const facebook::hermes::debugger::EvalResult &)>
         resultTransformer) {
-  std::lock_guard<std::mutex> lock(mutex_);
+  std::scoped_lock lock(mutex_);
 
   state_->pushPendingEval(
       frameIndex, src, promise, std::move(resultTransformer));
@@ -684,7 +685,7 @@ void Inspector::evaluateOnExecutor(
 void Inspector::setPauseOnExceptionsOnExecutor(
     const debugger::PauseOnThrowMode &mode,
     std::shared_ptr<folly::Promise<folly::Unit>> promise) {
-  std::lock_guard<std::mutex> local(mutex_);
+  std::scoped_lock local(mutex_);
 
   state_->pushPendingFunc([this, mode, promise] {
     debugger_.setPauseOnThrowMode(mode);

packages/react-native/ReactCommon/hermes/inspector/chrome/Connection.cpp

@@ -222,7 +222,7 @@ std::string Connection::Impl::getTitle() const {
 
 bool Connection::Impl::connect(std::unique_ptr<IRemoteConnection> remoteConn) {
   assert(remoteConn);
-  std::lock_guard<std::mutex> lock(connectionMutex_);
+  std::scoped_lock lock(connectionMutex_);
 
   if (connected_) {
     return false;
@@ -237,7 +237,7 @@ bool Connection::Impl::connect(std::unique_ptr<IRemoteConnection> remoteConn) {
 }
 
 bool Connection::Impl::disconnect() {
-  std::lock_guard<std::mutex> lock(connectionMutex_);
+  std::scoped_lock lock(connectionMutex_);
 
   if (!connected_) {
     return false;
@@ -350,7 +350,7 @@ void Connection::Impl::onPause(
       note.reason = "other";
       note.hitBreakpoints = std::vector<m::debugger::BreakpointId>();
 
-      std::lock_guard<std::mutex> lock(virtualBreakpointMutex_);
+      std::scoped_lock lock(virtualBreakpointMutex_);
       for (auto &bp :
            virtualBreakpoints_[kBeforeScriptWithSourceMapExecution]) {
         note.hitBreakpoints->emplace_back(bp);
@@ -398,7 +398,7 @@ void Connection::Impl::onScriptParsed(
   if (!info.sourceMappingUrl.empty()) {
     note.sourceMapURL = info.sourceMappingUrl;
 
-    std::lock_guard<std::mutex> lock(virtualBreakpointMutex_);
+    std::scoped_lock lock(virtualBreakpointMutex_);
     if (hasVirtualBreakpoint(kBeforeScriptWithSourceMapExecution)) {
       // We are precariously relying on the fact that onScriptParsed
       // is invoked immediately before the pause load mode is checked.
@@ -409,7 +409,7 @@ void Connection::Impl::onScriptParsed(
   }
 
   {
-    std::lock_guard<std::mutex> lock(parsedScriptsMutex_);
+    std::scoped_lock lock(parsedScriptsMutex_);
     parsedScripts_.push_back(info.fileName);
   }
 
@@ -1182,7 +1182,7 @@ void Connection::Impl::handle(const m::debugger::PauseRequest &req) {
 
 void Connection::Impl::handle(const m::debugger::RemoveBreakpointRequest &req) {
   if (isVirtualBreakpointId(req.breakpointId)) {
-    std::lock_guard<std::mutex> lock(virtualBreakpointMutex_);
+    std::scoped_lock lock(virtualBreakpointMutex_);
     if (!removeVirtualBreakpoint(req.breakpointId)) {
       sendErrorToClientViaExecutor(
           req.id, "Unknown breakpoint ID: " + req.breakpointId);
@@ -1238,7 +1238,7 @@ void Connection::Impl::handle(
   debugger::SourceLocation loc;
 
   {
-    std::lock_guard<std::mutex> lock(parsedScriptsMutex_);
+    std::scoped_lock lock(parsedScriptsMutex_);
     setHermesLocation(loc, req, parsedScripts_);
   }
 
@@ -1307,7 +1307,7 @@ void Connection::Impl::handle(
 
   // The act of creating and registering the breakpoint ID is enough
   // to "set" it. We merely check for the existence of them later.
-  std::lock_guard<std::mutex> lock(virtualBreakpointMutex_);
+  std::scoped_lock lock(virtualBreakpointMutex_);
   m::debugger::SetInstrumentationBreakpointResponse resp;
   resp.id = req.id;
   resp.breakpointId = createVirtualBreakpoint(req.instrumentation);

packages/react-native/ReactCommon/hermes/inspector/chrome/ConnectionDemux.cpp

@@ -65,7 +65,7 @@ ConnectionDemux::~ConnectionDemux() = default;
 DebugSessionToken ConnectionDemux::enableDebugging(
     std::unique_ptr<RuntimeAdapter> adapter,
     const std::string &title) {
-  std::lock_guard<std::mutex> lock(mutex_);
+  std::scoped_lock lock(mutex_);
 
   // TODO(#22976087): workaround for ComponentScript contexts never being
   // destroyed.
@@ -91,7 +91,7 @@ DebugSessionToken ConnectionDemux::enableDebugging(
 }
 
 void ConnectionDemux::disableDebugging(DebugSessionToken session) {
-  std::lock_guard<std::mutex> lock(mutex_);
+  std::scoped_lock lock(mutex_);
   if (conns_.find(session) == conns_.end()) {
     return;
   }

packages/react-native/ReactCommon/hermes/inspector/chrome/tests/ConnectionDemuxTests.cpp

@@ -58,7 +58,7 @@ class TestRemoteConnection : public IRemoteConnection {
     }
 
     void setDisconnected() {
-      std::lock_guard<std::mutex> lock(mutex_);
+      std::scoped_lock lock(mutex_);
       connected_ = false;
       cv_.notify_one();
     }

packages/react-native/ReactCommon/hermes/inspector/chrome/tests/SyncConnection.cpp

@@ -115,7 +115,7 @@ void SyncConnection::waitForNotification(
 void SyncConnection::onReply(const std::string &message) {
   LOG(INFO) << "SyncConnection::onReply got message: " << prettify(message);
 
-  std::lock_guard<std::mutex> lock(mutex_);
+  std::scoped_lock lock(mutex_);
 
   folly::dynamic obj = folly::parseJson(message);
   if (obj.count("id")) {