Fixes for the modem simulator

base/cvd/cuttlefish/host/commands/modem_simulator/call_service.cpp

@@ -90,6 +90,7 @@ std::vector<CommandHandler> CallService::InitializeCommandHandlers() {
 
 // This also resumes held calls
 void CallService::SimulatePendingCallsAnswered() {
+  std::lock_guard<std::mutex> lock(active_calls_mutex_);
   for (auto& iter : active_calls_) {
     if (iter.second.isCallDialing()) {
       iter.second.SetCallActive();
@@ -100,6 +101,7 @@ void CallService::SimulatePendingCallsAnswered() {
 void CallService::TimerWaitingRemoteCallResponse(CallToken call_token) {
   VLOG(0) << "Dialing id: " << call_token.first
           << ", number: " << call_token.second << "timeout, cancel";
+  std::lock_guard<std::mutex> lock(active_calls_mutex_);
   auto iter = active_calls_.find(call_token.first);
   if (iter != active_calls_.end() && iter->second.number == call_token.second) {
     if (iter->second.remote_client != std::nullopt) {
@@ -112,6 +114,7 @@ void CallService::TimerWaitingRemoteCallResponse(CallToken call_token) {
 
 /* ATD */
 void CallService::HandleDial(const Client& client, const std::string& command) {
+  std::lock_guard<std::mutex> lock(active_calls_mutex_);
   // Check the network registration state
   auto registration_state = NetworkService::NET_REGISTRATION_UNKNOWN;
   if (network_service_) {
@@ -265,6 +268,7 @@ void CallService::SendCallStatusToRemote(CallStatus& call,
 
 /* ATA */
 void CallService::HandleAcceptCall(const Client& client) {
+  std::lock_guard<std::mutex> lock(active_calls_mutex_);
   for (auto& iter : active_calls_) {
     if (iter.second.isCallIncoming()) {
       iter.second.SetCallActive();
@@ -280,6 +284,7 @@ void CallService::HandleAcceptCall(const Client& client) {
 
 /* ATH */
 void CallService::HandleRejectCall(const Client& client) {
+  std::lock_guard<std::mutex> lock(active_calls_mutex_);
   for (auto iter = active_calls_.begin(); iter != active_calls_.end();) {
     /* ATH: hangup, since user is busy */
     if (iter->second.isCallIncoming()) {
@@ -339,6 +344,7 @@ void CallService::HandleRejectCall(const Client& client) {
  *see RIL_REQUEST_GET_CURRENT_CALLS in RIL
  */
 void CallService::HandleCurrentCalls(const Client& client) {
+  std::lock_guard<std::mutex> lock(active_calls_mutex_);
   std::vector<std::string> responses;
   std::stringstream ss;
 
@@ -396,12 +402,13 @@ void CallService::HandleCurrentCalls(const Client& client) {
  */
 void CallService::HandleHangup(const Client& client,
                                const std::string& command) {
+  std::lock_guard<std::mutex> lock(active_calls_mutex_);
   std::vector<std::string> responses;
   CommandParser cmd(command);
   cmd.SkipPrefix();
 
-  std::string action(*cmd);
-  int n = std::stoi(action.substr(0, 1));
+  const std::string action(*cmd);
+  const int n = action.empty() ? -1 : (action[0] - '0');
   int index = -1;
   if (cmd->length() > 1) {
     index = std::stoi(action.substr(1));
@@ -643,6 +650,7 @@ void CallService::CallStateUpdate() {
  */
 void CallService::HandleRemoteCall(const Client& client,
                                    const std::string& command) {
+  std::lock_guard<std::mutex> lock(active_calls_mutex_);
   CommandParser cmd(command);
   cmd.SkipPrefix();
 

base/cvd/cuttlefish/host/commands/modem_simulator/call_service.h

@@ -15,6 +15,8 @@
 
 #pragma once
 
+#include <mutex>
+
 #include "cuttlefish/host/commands/modem_simulator/modem_service.h"
 #include "cuttlefish/host/commands/modem_simulator/network_service.h"
 #include "cuttlefish/host/commands/modem_simulator/sim_service.h"
@@ -32,6 +34,7 @@ class CallService : public ModemService, public std::enable_shared_from_this<Cal
 
   void SetupDependency(SimService* sim, NetworkService* net);
 
+ private:
   void HandleDial(const Client& client, const std::string& command);
   void HandleAcceptCall(const Client& client);
   void HandleRejectCall(const Client& client);
@@ -50,7 +53,6 @@ class CallService : public ModemService, public std::enable_shared_from_this<Cal
     }
   }
 
- private:
   void InitializeServiceState();
   std::vector<CommandHandler> InitializeCommandHandlers();
   void SimulatePendingCallsAnswered();
@@ -148,6 +150,7 @@ class CallService : public ModemService, public std::enable_shared_from_this<Cal
   // private data members
   SimService* sim_service_;
   NetworkService* network_service_;
+  std::mutex active_calls_mutex_;
   std::map<int, CallStatus> active_calls_;
   bool in_emergency_mode_;
   bool mute_on_;

base/cvd/cuttlefish/host/commands/modem_simulator/channel_monitor.cpp

@@ -46,16 +46,15 @@ ClientId ChannelMonitor::SetRemoteClient(SharedFD client, bool is_accepted) {
   if (is_accepted) {
     // There may be new data from remote client before select.
     remote_client->first_read_command_ = true;
-    ReadCommand(*remote_client);
   }
 
-  if (remote_client->client_read_fd_->IsOpen() &&
-      remote_client->client_write_fd_->IsOpen()) {
-    remote_client->first_read_command_ = false;
-    remote_clients_.push_back(std::move(remote_client));
-    VLOG(0) << "added one remote client";
+  {
+    std::lock_guard<std::mutex> lock(clients_mutex_);
+    pending_remote_.push_back(std::move(remote_client));
   }
 
+  VLOG(0) << "added one remote client";
+
   // Trigger monitor loop
   if (write_pipe_->IsOpen()) {
     write_pipe_->Write("OK", sizeof("OK"));
@@ -83,8 +82,8 @@ void ChannelMonitor::AcceptIncomingConnection() {
 }
 
 void ChannelMonitor::ReadCommand(Client& client) {
-  std::vector<char> buffer(kMaxCommandLength);
-  auto bytes_read = client.client_read_fd_->Read(buffer.data(), buffer.size());
+  char buffer[kMaxCommandLength];
+  auto bytes_read = client.client_read_fd_->Read(buffer, kMaxCommandLength);
   if (bytes_read <= 0) {
     if (errno == EAGAIN && client.type == Client::REMOTE &&
         client.first_read_command_) {
@@ -96,24 +95,18 @@ void ChannelMonitor::ReadCommand(Client& client) {
             << client.client_read_fd_->StrError();
     client.client_read_fd_->Close();  // Ignore errors here
     client.client_write_fd_->Close();
-    // Erase client from the vector clients
-    auto& clients = client.type == Client::REMOTE ? remote_clients_ : clients_;
-    auto iter = std::find_if(
-        clients.begin(), clients.end(),
-        [&](std::unique_ptr<Client>& other) { return *other == client; });
-    if (iter != clients.end()) {
-      clients.erase(iter);
-    }
+
+    // Do NOT erase here — we are called from inside MonitorLoop's range-for
+    // over the same vector.  Mark invalid and erase in removeInvalidClients
+    // outside of the loop.
+    client.is_valid = false;
     return;
   }
-
-  std::string& incomplete_command = client.incomplete_command;
+  client.first_read_command_ = false;
 
   // Add the incomplete command from the last read
-  auto commands = std::string{incomplete_command.data()};
-  commands.append(buffer.data());
-
-  incomplete_command.clear();
+  std::string commands(std::move(client.incomplete_command));
+  commands.append(buffer, bytes_read);
 
   // Replacing '\n' with '\r'
   absl::StrReplaceAll({{"\n", "\r"}}, &commands);
@@ -134,14 +127,15 @@ void ChannelMonitor::ReadCommand(Client& client) {
       }
       pos = r_pos + 1;                     // Skip '\r'
     } else if (pos < commands.length()) {  // Incomplete command
-      incomplete_command = commands.substr(pos);
-      VLOG(1) << "incomplete command: " << incomplete_command;
+      client.incomplete_command = commands.substr(pos);
+      VLOG(1) << "incomplete command: " << client.incomplete_command;
     }
   }
 }
 
 void ChannelMonitor::SendUnsolicitedCommand(std::string& response) {
   // The first accepted client default to be unsolicited command channel?
+  std::lock_guard<std::mutex> lock(clients_mutex_);
   auto iter = clients_.begin();
   if (iter != clients_.end()) {
     iter->get()->SendCommandResponse(response);
@@ -151,6 +145,7 @@ void ChannelMonitor::SendUnsolicitedCommand(std::string& response) {
 }
 
 void ChannelMonitor::SendRemoteCommand(ClientId client, std::string& response) {
+  std::lock_guard<std::mutex> lock(clients_mutex_);
   auto iter = remote_clients_.begin();
   for (; iter != remote_clients_.end(); ++iter) {
     if (iter->get()->Id() == client) {
@@ -162,6 +157,7 @@ void ChannelMonitor::SendRemoteCommand(ClientId client, std::string& response) {
 }
 
 void ChannelMonitor::CloseRemoteConnection(ClientId client) {
+  std::lock_guard<std::mutex> lock(clients_mutex_);
   auto iter = remote_clients_.begin();
   for (; iter != remote_clients_.end(); ++iter) {
     if (iter->get()->Id() == client) {
@@ -211,16 +207,27 @@ void ChannelMonitor::MonitorLoop() {
     cuttlefish::SharedFDSet read_set;
     read_set.Set(server_);
     read_set.Set(read_pipe_);
-    for (auto& client : clients_) {
-      if (client->is_valid) {
-        read_set.Set(client->client_read_fd_);
+
+    {
+      std::lock_guard<std::mutex> lock(clients_mutex_);
+      remote_clients_.insert(
+          remote_clients_.end(),
+          std::make_move_iterator(pending_remote_.begin()),
+          std::make_move_iterator(pending_remote_.end()));
+      pending_remote_.clear();
+
+      for (auto& client : clients_) {
+        if (client->is_valid) {
+          read_set.Set(client->client_read_fd_);
+        }
       }
-    }
-    for (auto& client : remote_clients_) {
-      if (client->is_valid) {
-        read_set.Set(client->client_read_fd_);
+      for (auto& client : remote_clients_) {
+        if (client->is_valid) {
+          read_set.Set(client->client_read_fd_);
+        }
       }
     }
+
     int num_fds = cuttlefish::Select(&read_set, nullptr, nullptr, nullptr);
     if (num_fds < 0) {
       LOG(ERROR) << "Select call returned error : " << strerror(errno);
@@ -237,20 +244,22 @@ void ChannelMonitor::MonitorLoop() {
           VLOG(0) << "requested to exit now";
           break;
         }
-        // clean the lists
-        removeInvalidClients(clients_);
-        removeInvalidClients(remote_clients_);
       }
+
+      std::lock_guard<std::mutex> lock(clients_mutex_);
       for (auto& client : clients_) {
-        if (read_set.IsSet(client->client_read_fd_)) {
+        if (client->is_valid && read_set.IsSet(client->client_read_fd_)) {
           ReadCommand(*client);
         }
       }
       for (auto& client : remote_clients_) {
-        if (read_set.IsSet(client->client_read_fd_)) {
+        if (client->is_valid && read_set.IsSet(client->client_read_fd_)) {
           ReadCommand(*client);
         }
       }
+
+      removeInvalidClients(clients_);
+      removeInvalidClients(remote_clients_);
     } else {
       // Ignore errors here
       LOG(ERROR) << "Select call returned error : " << strerror(errno);

base/cvd/cuttlefish/host/commands/modem_simulator/channel_monitor.h

@@ -15,6 +15,7 @@
 
 #pragma once
 
+#include <mutex>
 #include <thread>
 #include <vector>
 
@@ -53,8 +54,10 @@ class ChannelMonitor {
   cuttlefish::SharedFD server_;
   cuttlefish::SharedFD read_pipe_;
   cuttlefish::SharedFD write_pipe_;
+  std::mutex clients_mutex_;
   std::vector<std::unique_ptr<Client>> clients_;
   std::vector<std::unique_ptr<Client>> remote_clients_;
+  std::vector<std::unique_ptr<Client>> pending_remote_;
 
   void AcceptIncomingConnection();
   void OnClientSocketClosed(int sock);

base/cvd/cuttlefish/host/commands/modem_simulator/client.h

@@ -78,7 +78,7 @@ class Client {
   SharedFD client_write_fd_;
   std::string incomplete_command;
   mutable std::mutex write_mutex;
-  bool first_read_command_;  // Only used when ClientType::REMOTE
+  bool first_read_command_ = false;  // Only used when ClientType::REMOTE
   bool is_valid = true;
 };
 

base/cvd/cuttlefish/host/commands/modem_simulator/modem_service.cpp

@@ -34,14 +34,13 @@ CommandHandler::CommandHandler(const std::string& command, p_func handler)
       p_command_handler(handler) {}
 
 int CommandHandler::Compare(const std::string& command) const {
-  int result = -1;
-  if (match_mode == PARTIAL_MATCH) {
-    result =
-        command.compare(2, command_prefix.size(), command_prefix);  // skip "AT"
-  } else {
-    result = command.compare(2, command.size(), command_prefix);
+  if (command.size() < 2) {  // the "AT" prefix
+    return -1;
   }
-  return result;
+
+  return (match_mode == PARTIAL_MATCH)
+             ? command.compare(2, command_prefix.size(), command_prefix)
+             : command.compare(2, command.size(), command_prefix);
 }
 
 void CommandHandler::HandleCommand(const Client& client,

base/cvd/cuttlefish/host/commands/modem_simulator/sms_service.cpp

@@ -297,7 +297,7 @@ void SmsService::HandleSendSMSPDU(const Client& client, std::string& command) {
       thread_looper_->Post(
           makeSafeCallback<SmsService>(
               this,
-              [&sms_pdu](SmsService* me) { me->HandleReceiveSMS(sms_pdu); }),
+              [sms_pdu](SmsService* me) { me->HandleReceiveSMS(sms_pdu); }),
           std::chrono::seconds(1));
     } else {  // Send SMS to remote host port
       SendSmsToRemote(remote_host_port, sms_pdu);