Make the random number generation and connection id thread-safe.

Given the random number generation and connection id are both closely tied up with write logic and creation of reactor_, so it would make sense to re-use `reactor_mtx_` instead of generating another dedicated lock.

Tested with 5000 runs with this CL with TSAN finding nothing.

PiperOrigin-RevId: 692219060

Author: GuestPlatform Team

cpp/acs_agent_client.cc

@@ -2,7 +2,9 @@
 
 #include <algorithm>
 #include <chrono>
+#include <cstdint>
 #include <future>
+#include <limits>
 #include <memory>
 #include <queue>
 #include <string>
@@ -15,6 +17,7 @@
 #include "third_party/absl/functional/bind_front.h"
 #include "third_party/absl/log/absl_log.h"
 #include "third_party/absl/memory/memory.h"
+#include "third_party/absl/random/distributions.h"
 #include "third_party/absl/status/status.h"
 #include "third_party/absl/status/statusor.h"
 #include "third_party/absl/strings/str_cat.h"
@@ -83,7 +86,10 @@ absl::Status AcsAgentClient::AddRequest(Request& request) {
   // TODO: Make the retry parameters configurable.
   for (int i = 0; i < 5; ++i) {
     // Generate a new message id for each attempt.
-    request.set_message_id(CreateMessageUuid());
+    {
+      absl::MutexLock lock(&reactor_mtx_);
+      request.set_message_id(CreateMessageUuid());
+    }
     latest_send_request_status = AddRequestAndWaitForResponse(request);
     if (latest_send_request_status.ok()) {
       return absl::OkStatus();
@@ -454,10 +460,9 @@ void AcsAgentClient::Shutdown() {
 }
 
 std::string AcsAgentClient::CreateMessageUuid() {
-  // TODO: b/376530555 - Make the random number generation thread-safe. The gen_
-  // is not thread-safe. int64_t random =
-  //     absl::Uniform<int64_t>(gen_, 0, std::numeric_limits<int64_t>::max());
-  return absl::StrCat(absl::ToUnixMicros(absl::Now()));
+  int64_t random =
+      absl::Uniform<int64_t>(gen_, 0, std::numeric_limits<int64_t>::max());
+  return absl::StrCat(random, "-", absl::ToUnixMicros(absl::Now()));
 }
 
 void AcsAgentClient::SetValueAndRemovePromise(const std::string& message_id,

cpp/acs_agent_client.h

@@ -158,13 +158,13 @@ class AcsAgentClient {
   // the restart_client_thread_.
   std::unique_ptr<
       google::cloud::agentcommunication::v1::AgentCommunication::Stub>
-  GenerateConnectionIdAndStub();
+  GenerateConnectionIdAndStub() ABSL_EXCLUSIVE_LOCKS_REQUIRED(reactor_mtx_);
 
   // Creates a unique message id. Currently, it is "{random
   // number}-{current_timestamp}". The requirement of the uniqueness from ACS
   // server is not very strict: We just need to make sure the message id is
   // unique within a couple of seconds for each agent.
-  std::string CreateMessageUuid();
+  std::string CreateMessageUuid() ABSL_EXCLUSIVE_LOCKS_REQUIRED(reactor_mtx_);
 
   // Set the value of the promise and remove the promise from the map. This
   // function is used to ensure:
@@ -176,17 +176,12 @@ class AcsAgentClient {
                                 absl::Status status)
       ABSL_EXCLUSIVE_LOCKS_REQUIRED(request_delivery_status_mtx_);
 
-  // TODO: b/376530555 - Make the random number generation thread-safe. The gen_
-  // is not thread-safe, needs to be protected by a mutex. Random number
-  // generator for creating message id.
-  absl::BitGen gen_;
+  absl::BitGen gen_ ABSL_GUARDED_BY(reactor_mtx_);
 
   // Dedicated to reading the Response from the server.
   std::thread read_response_thread_;
 
-  // TODO: b/376530555 - Make the connection id thread-safe.
-  // Connection id of the agent.
-  AgentConnectionId connection_id_;
+  AgentConnectionId connection_id_ ABSL_GUARDED_BY(reactor_mtx_);
 
   // Mutex to protect the reactor_ and other variables that are only accessed
   // by the reactor_ for write operations.