bmahler commented on code in PR #570:
URL: https://github.com/apache/mesos/pull/570#discussion_r1578248075
##########
src/linux/cgroups2.cpp:
##########
@@ -487,6 +487,27 @@ Try<set<pid_t>> processes(const string& cgroup)
pids.insert(*pid);
}
+ if (!recursive) {
+ return pids;
+ }
+
+ Try<set<string>> cgroups = cgroups2::get(cgroup);
+ foreach (const string& cgroup, *cgroups) {
+ Try<set<pid_t>> _pids = cgroups2::processes(cgroup);
+ if (_pids.isError() && cgroups2::exists(cgroup)) {
+ return Error(
+ "Failed to get processes in '" + cgroup + "': " + _pids.error());
+ } else if (_pids.isError()) {
+ // The cgroup was just removed. Ignore the error and treat it as if
+ // the cgroup had no processes.
+ continue;
Review Comment:
this is going to ignore parse failures as well since you're looking at any
error from the processes call, would be better to check this on read
##########
src/linux/cgroups2.cpp:
##########
@@ -487,6 +487,27 @@ Try<set<pid_t>> processes(const string& cgroup)
pids.insert(*pid);
}
+ if (!recursive) {
+ return pids;
+ }
+
+ Try<set<string>> cgroups = cgroups2::get(cgroup);
+ foreach (const string& cgroup, *cgroups) {
Review Comment:
this can crash, naked de-reference without handling the error case
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
