Skip to content

Commit 58840df

Browse files
committed
Actually kill all run_cvd processes from cvd reset
It was failing to do so before because it was trying to extract more information than just the pid from the process and giving up when it couldn't. That extra information isn't needed to kill it.
1 parent 215adec commit 58840df

File tree

6 files changed

+73
-14
lines changed

6 files changed

+73
-14
lines changed

base/cvd/cuttlefish/common/libs/utils/files.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,14 @@ off_t FileSize(const std::string& path) {
450450
return st.st_size;
451451
}
452452

453+
Result<uid_t> FileOwner(const std::string& path) {
454+
struct stat st{};
455+
if (stat(path.c_str(), &st) == -1) {
456+
return CF_ERRF("Failed to stat file '{}' : {}", path, StrError(errno));
457+
}
458+
return st.st_uid;
459+
}
460+
453461
bool MakeFileExecutable(const std::string& path) {
454462
LOG(DEBUG) << "Making " << path << " executable";
455463
return chmod(path.c_str(), S_IRWXU) == 0;

base/cvd/cuttlefish/common/libs/utils/files.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ Result<std::chrono::system_clock::time_point> FileModificationTime(
7878
const std::string& path);
7979
// Whether a file exists and is a unix socket
8080
bool FileIsSocket(const std::string& path);
81+
Result<uid_t> FileOwner(const std::string& path);
8182

8283
// acloud related API
8384
std::string FindImage(const std::string& search_path,

base/cvd/cuttlefish/host/commands/cvd/instances/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,7 @@ cf_cc_library(
175175
srcs = ["stop.cpp"],
176176
hdrs = ["stop.h"],
177177
deps = [
178+
"//cuttlefish/common/libs/posix:strerror",
178179
"//cuttlefish/common/libs/utils:contains",
179180
"//cuttlefish/common/libs/utils:files",
180181
"//cuttlefish/common/libs/utils:proc_file_utils",

base/cvd/cuttlefish/host/commands/cvd/instances/run_cvd_proc_collector.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,4 +236,8 @@ Result<std::vector<GroupProcInfo>> CollectRunCvdGroups() {
236236
return collector.CfGroups();
237237
}
238238

239+
Result<std::vector<pid_t>> CollectRunCvdProcesses() {
240+
return CF_EXPECT(CollectPidsByExecName("run_cvd", getuid()));
241+
}
242+
239243
} // namespace cuttlefish

base/cvd/cuttlefish/host/commands/cvd/instances/run_cvd_proc_collector.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,4 +51,6 @@ struct GroupProcInfo {
5151

5252
Result<std::vector<GroupProcInfo>> CollectRunCvdGroups();
5353

54+
Result<std::vector<pid_t>> CollectRunCvdProcesses();
55+
5456
} // namespace cuttlefish

base/cvd/cuttlefish/host/commands/cvd/instances/stop.cpp

Lines changed: 57 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@
1616

1717
#include "cuttlefish/host/commands/cvd/instances/stop.h"
1818

19+
#include <errno.h>
1920
#include <signal.h>
21+
#include <string.h>
22+
#include <unistd.h>
2023

2124
#include <iostream> // std::endl
2225
#include <sstream>
@@ -30,6 +33,7 @@
3033
#include <fmt/core.h>
3134
#include <fmt/ranges.h> // NOLINT(misc-include-cleaner): version difference
3235

36+
#include "cuttlefish/common/libs/posix/strerror.h"
3337
#include "cuttlefish/common/libs/utils/contains.h"
3438
#include "cuttlefish/common/libs/utils/files.h"
3539
#include "cuttlefish/common/libs/utils/proc_file_utils.h"
@@ -110,6 +114,12 @@ static bool IsStillRunCvd(const pid_t pid) {
110114
"run_cvd");
111115
}
112116

117+
Result<void> SendSignal(pid_t pid) {
118+
int kill_res = kill(pid, SIGKILL);
119+
CF_EXPECTF(kill_res == 0, "Failed to kill {}: {}", pid, StrError(errno));
120+
return {};
121+
}
122+
113123
Result<void> SendSignal(const GroupProcInfo& group_info) {
114124
std::vector<pid_t> failed_pids;
115125
for (const auto& [unused, instance] : group_info.instances_) {
@@ -118,7 +128,7 @@ Result<void> SendSignal(const GroupProcInfo& group_info) {
118128
continue;
119129
}
120130
LOG(INFO) << "Sending SIGKILL to process " << parent_run_cvd_pid;
121-
if (kill(parent_run_cvd_pid, SIGKILL) == 0) {
131+
if (SendSignal(parent_run_cvd_pid).ok()) {
122132
LOG(VERBOSE) << "Successfully SIGKILL'ed " << parent_run_cvd_pid;
123133
} else {
124134
failed_pids.push_back(parent_run_cvd_pid);
@@ -170,25 +180,58 @@ Result<void> ForcefullyStopGroup(const GroupProcInfo& group) {
170180
return {};
171181
}
172182

183+
Result<void> KillAllRunCvds() {
184+
auto run_cvd_pids = CF_EXPECT(CollectRunCvdProcesses());
185+
if (run_cvd_pids.empty()) {
186+
return {};
187+
}
188+
LOG(INFO) << run_cvd_pids.size()
189+
<< " run_cvd processes still remain, will stop forcefully";
190+
for (pid_t group_pid : run_cvd_pids) {
191+
if (Result<void> result = SendSignal(group_pid); !result.ok()) {
192+
LOG(ERROR) << result.error().FormatForEnv();
193+
}
194+
}
195+
return {};
196+
}
197+
198+
Result<void> DeleteAllOwnedInstanceLocks() {
199+
const std::string lock_dir = InstanceLocksPath();
200+
uid_t own_uid = geteuid();
201+
for (const std::string& lock_file: CF_EXPECT(DirectoryContents(lock_dir))) {
202+
std::string lock_file_path = fmt::format("{}/{}", lock_dir, lock_file);
203+
Result<uid_t> file_uid_res = FileOwner(lock_file_path);
204+
if (!file_uid_res.ok()) {
205+
LOG(ERROR) << "Failed to obtain owner of '" << lock_file_path << "'";
206+
continue;
207+
}
208+
if (*file_uid_res != own_uid) {
209+
LOG(VERBOSE) << "Skipped '" << lock_file_path
210+
<< "' because it's not owned by current user";
211+
continue;
212+
}
213+
if (!RemoveFile(lock_file_path)) {
214+
LOG(ERROR) << "Failed to remove '" << lock_file_path << "'";
215+
}
216+
}
217+
return {};
218+
}
219+
173220
} // namespace
174221

175222
Result<void> KillAllCuttlefishInstances(bool clear_runtime_dirs) {
176-
auto stop_cvd_result = RunStopCvdAll(clear_runtime_dirs);
177-
if (!stop_cvd_result.ok()) {
178-
LOG(ERROR) << stop_cvd_result.error().FormatForEnv();
223+
if (Result<void> res = RunStopCvdAll(clear_runtime_dirs); !res.ok()) {
224+
LOG(ERROR) << res.error().FormatForEnv();
179225
}
180-
std::vector<GroupProcInfo> group_infos = CF_EXPECT(CollectRunCvdGroups());
181-
if (group_infos.empty()) {
182-
return {};
226+
227+
if (Result<void> res = KillAllRunCvds(); !res.ok()) {
228+
LOG(ERROR) << res.error().FormatForEnv();
183229
}
184-
LOG(INFO) << group_infos.size()
185-
<< " instance groups still remain, will stop forcefully";
186-
for (const GroupProcInfo& group_info : group_infos) {
187-
auto result = ForcefullyStopGroup(group_info);
188-
if (!result.ok()) {
189-
LOG(ERROR) << result.error().FormatForEnv();
190-
}
230+
231+
if (Result<void> res = DeleteAllOwnedInstanceLocks(); !res.ok()) {
232+
LOG(ERROR) << res.error().FormatForEnv();
191233
}
234+
192235
return {};
193236
}
194237

0 commit comments

Comments
 (0)