Skip to content

Commit 6a48b31

Browse files
Fix Cuttlefish multi-instance restart deadlocks, socket hangs, and UWB/bootconfig configuration mismatches
1 parent d9af5a9 commit 6a48b31

5 files changed

Lines changed: 24 additions & 13 deletions

File tree

base/cvd/cuttlefish/common/libs/fs/shared_fd.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -518,14 +518,15 @@ int SharedFD::Fchdir(SharedFD shared_fd) {
518518

519519
Result<SharedFD> SharedFD::Fifo(const std::string& path, mode_t mode) {
520520
struct stat st {};
521-
if (TEMP_FAILURE_RETRY(stat(path.c_str(), &st)) == 0) {
522-
CF_EXPECTF(TEMP_FAILURE_RETRY(remove(path.c_str())) == 0,
523-
"Failed to delete old file at '{}': '{}'", path,
521+
if (TEMP_FAILURE_RETRY(stat(path.c_str(), &st)) != 0) {
522+
CF_EXPECTF(TEMP_FAILURE_RETRY(mkfifo(path.c_str(), mode)) == 0,
523+
"Failed to mkfifo('{}', {:o}): {}", path, mode,
524524
::cuttlefish::StrError(errno));
525+
} else {
526+
CF_EXPECTF(S_ISFIFO(st.st_mode),
527+
"File at '{}' exists but is not a FIFO", path);
525528
}
526529

527-
CF_EXPECTF(TEMP_FAILURE_RETRY(mkfifo(path.c_str(), mode)) == 0,
528-
"Failed to mkfifo('{}', {:o})", path, mode);
529530
auto ret = Open(path, O_RDWR);
530531
CF_EXPECTF(ret->IsOpen(), "Failed to open '{}': '{}'", path, ret->StrError());
531532
return ret;

base/cvd/cuttlefish/host/commands/run_cvd/launch/uwb_connector.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ namespace cuttlefish {
3939
Result<std::optional<MonitorCommand>> UwbConnector(
4040
const CuttlefishConfig& config,
4141
const CuttlefishConfig::InstanceSpecific& instance) {
42-
if (!instance.enable_host_uwb_connector()) {
42+
if (!config.enable_host_uwb()) {
4343
return {};
4444
}
4545
std::vector<std::string> fifo_paths = {
@@ -50,6 +50,9 @@ Result<std::optional<MonitorCommand>> UwbConnector(
5050
for (const auto& path : fifo_paths) {
5151
fifos.push_back(CF_EXPECT(SharedFD::Fifo(path, 0660)));
5252
}
53+
if (!instance.enable_host_uwb_connector()) {
54+
return {};
55+
}
5356
return Command(HostBinaryPath("tcp_connector"))
5457
.AddParameter("-fifo_out=", fifos[0])
5558
.AddParameter("-fifo_in=", fifos[1])

base/cvd/cuttlefish/host/commands/run_cvd/server_loop_impl.cpp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -337,12 +337,6 @@ void ServerLoopImpl::DeleteFifos() {
337337
instance_.PerInstanceInternalPath("gatekeeper_fifo_vm.out"),
338338
instance_.PerInstanceInternalPath("oemlock_fifo_vm.in"),
339339
instance_.PerInstanceInternalPath("oemlock_fifo_vm.out"),
340-
instance_.PerInstanceInternalPath("bt_fifo_vm.in"),
341-
instance_.PerInstanceInternalPath("bt_fifo_vm.out"),
342-
instance_.PerInstanceInternalPath("nfc_fifo_vm.in"),
343-
instance_.PerInstanceInternalPath("nfc_fifo_vm.out"),
344-
instance_.PerInstanceInternalPath("uwb_fifo_vm.in"),
345-
instance_.PerInstanceInternalPath("uwb_fifo_vm.out"),
346340
instance_.PerInstanceInternalPath("gnsshvc_fifo_vm.in"),
347341
instance_.PerInstanceInternalPath("gnsshvc_fifo_vm.out"),
348342
instance_.PerInstanceInternalPath("locationhvc_fifo_vm.in"),

base/cvd/cuttlefish/host/libs/process_monitor/process_monitor.cc

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,15 @@ Result<void> ProcessMonitor::StartSubprocesses(
278278
Result<void> ProcessMonitor::ReadMonitorSocketLoop(std::atomic_bool& running) {
279279
VLOG(0) << "Waiting for a `stop` message from the parent";
280280
while (running.load()) {
281-
ManagedMessage message = CF_EXPECT(child_channel_->ReceiveMessage());
281+
auto message_res = child_channel_->ReceiveMessage();
282+
if (!message_res.ok()) {
283+
if (!running.load()) {
284+
// We were shut down intentionally, ignore error
285+
return {};
286+
}
287+
CF_EXPECT(std::move(message_res));
288+
}
289+
auto message = std::move(*message_res);
282290
if (message->command == ParentToChildMessageType::kStop) {
283291
running.store(false);
284292
// Wake up the wait() loop by giving it an exited child process
@@ -406,6 +414,7 @@ Result<void> ProcessMonitor::StartAndMonitorProcesses() {
406414
if (monitor_ == 0) {
407415
pipe_read->Close();
408416
child_channel_ = transport::SharedFdChannel(child_sock, child_sock);
417+
child_sock_ = std::move(child_sock);
409418
Result<void> monitor_result = MonitorRoutine();
410419
if (!monitor_result.ok()) {
411420
LOG(ERROR) << "Monitoring processes failed:\n" << monitor_result.error();
@@ -444,6 +453,9 @@ Result<void> ProcessMonitor::MonitorRoutine() {
444453
CF_EXPECT(MonitorLoop(running, properties_mutex_,
445454
properties_.restart_subprocesses_,
446455
properties_.entries_));
456+
if (child_sock_->IsOpen()) {
457+
child_sock_->Shutdown(SHUT_RDWR);
458+
}
447459
CF_EXPECT(parent_comms.get(), "Should have exited if monitoring stopped");
448460

449461
CF_EXPECT(StopSubprocesses(properties_.entries_));

base/cvd/cuttlefish/host/libs/process_monitor/process_monitor.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ class ProcessMonitor {
9494
pid_t monitor_;
9595
std::optional<transport::SharedFdChannel> parent_channel_;
9696
std::optional<transport::SharedFdChannel> child_channel_;
97+
SharedFD child_sock_;
9798

9899
/*
99100
* The lock that should be acquired when multiple threads

0 commit comments

Comments
 (0)