Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a049a992 authored by Inseob Kim's avatar Inseob Kim
Browse files

Kill services even when cgroups is disabled 

process_cgroup_empty_ is used to indicate that a service is already
killed or not. If cgroup support lacks, services cannot be killed
because process_cgroup_empty_ is always true.

This change fixes it by not assigning process_cgroup_empty_ as true.
Instead, make KillProcessGroup send signals even when cgroup is
disabled. Also DoKillProcessGroupOnce() is updated so it returns a number of killed processes, excluding already dead processes. This behavior agrees with its name (DoKillProcessOnce), and it prevents regression upon missing cgroups, because kill(-pgid) will always
"succeed" so KillProcessGroup will loop even when all processes are
already dead.

Bug: 257264124
Test: boot microdroid, see services are terminated
Change-Id: I19abf19ff1b70c666cd6f12d0a12956765174aaa
parent fa4926a9

init/service.cpp

+0 −2
Original line number Diff line number Diff line
@@ -711,8 +711,6 @@ Result<void> Service::Start() { 
        if (use_memcg) {

            ConfigureMemcg();

        }

    } else {

        process_cgroup_empty_ = true;

    }



    if (oom_score_adjust_ != DEFAULT_OOM_SCORE_ADJUST) {

libprocessgroup/processgroup.cpp

+59 −39
Original line number Diff line number Diff line
@@ -367,8 +367,17 @@ err: 
// Returns 0 if there are no processes in the process cgroup left to kill

// Returns -1 on error

static int DoKillProcessGroupOnce(const char* cgroup, uid_t uid, int initialPid, int signal) {

    // We separate all of the pids in the cgroup into those pids that are also the leaders of

    // process groups (stored in the pgids set) and those that are not (stored in the pids set).

    std::set<pid_t> pgids;

    pgids.emplace(initialPid);

    std::set<pid_t> pids;



    std::unique_ptr<FILE, decltype(&fclose)> fd(nullptr, fclose);



    if (CgroupsAvailable()) {

        auto path = ConvertUidPidToPath(cgroup, uid, initialPid) + PROCESSGROUP_CGROUP_PROCS_FILE;

    std::unique_ptr<FILE, decltype(&fclose)> fd(fopen(path.c_str(), "re"), fclose);

        fd.reset(fopen(path.c_str(), "re"));

        if (!fd) {

            if (errno == ENOENT) {

                // This happens when process is already dead
@@ -378,21 +387,15 @@ static int DoKillProcessGroupOnce(const char* cgroup, uid_t uid, int initialPid, 
                          << initialPid;

            return -1;

        }



    // We separate all of the pids in the cgroup into those pids that are also the leaders of

    // process groups (stored in the pgids set) and those that are not (stored in the pids set).

    std::set<pid_t> pgids;

    pgids.emplace(initialPid);

    std::set<pid_t> pids;



        pid_t pid;

    int processes = 0;

        bool file_is_empty = true;

        while (fscanf(fd.get(), "%d\n", &pid) == 1 && pid >= 0) {

        processes++;

            file_is_empty = false;

            if (pid == 0) {

                // Should never happen...  but if it does, trying to kill this

                // will boomerang right back and kill us!  Let's not let that happen.

            LOG(WARNING) << "Yikes, we've been told to kill pid 0!  How about we don't do that?";

                LOG(WARNING)

                        << "Yikes, we've been told to kill pid 0!  How about we don't do that?";

                continue;

            }

            pid_t pgid = getpgid(pid);
@@ -403,6 +406,10 @@ static int DoKillProcessGroupOnce(const char* cgroup, uid_t uid, int initialPid, 
                pids.emplace(pid);

            }

        }

        if (file_is_empty) {

            // This happens when process is already dead

            return 0;

        }



        // Erase all pids that will be killed when we kill the process groups.

        for (auto it = pids.begin(); it != pids.end();) {
@@ -413,13 +420,17 @@ static int DoKillProcessGroupOnce(const char* cgroup, uid_t uid, int initialPid, 
                ++it;

            }

        }

    }



    int processes = 0;

    // Kill all process groups.

    for (const auto pgid : pgids) {

        LOG(VERBOSE) << "Killing process group " << -pgid << " in uid " << uid

                     << " as part of process cgroup " << initialPid;



        if (kill(-pgid, signal) == -1 && errno != ESRCH) {

        if (kill(-pgid, signal) == 0) {

            processes++;

        } else if (errno != ESRCH) {

            PLOG(WARNING) << "kill(" << -pgid << ", " << signal << ") failed";

        }

    }
@@ -429,18 +440,22 @@ static int DoKillProcessGroupOnce(const char* cgroup, uid_t uid, int initialPid, 
        LOG(VERBOSE) << "Killing pid " << pid << " in uid " << uid << " as part of process cgroup "

                     << initialPid;



        if (kill(pid, signal) == -1 && errno != ESRCH) {

        if (kill(pid, signal) == 0) {

            processes++;

        } else if (errno != ESRCH) {

            PLOG(WARNING) << "kill(" << pid << ", " << signal << ") failed";

        }

    }



    return feof(fd.get()) ? processes : -1;

    return (!fd || feof(fd.get())) ? processes : -1;

}



static int KillProcessGroup(uid_t uid, int initialPid, int signal, int retries,

                            int* max_processes) {

    std::string hierarchy_root_path;

    if (CgroupsAvailable()) {

        CgroupGetControllerPath(CGROUPV2_CONTROLLER_NAME, &hierarchy_root_path);

    }

    const char* cgroup = hierarchy_root_path.c_str();



    std::chrono::steady_clock::time_point start = std::chrono::steady_clock::now();
@@ -485,6 +500,11 @@ static int KillProcessGroup(uid_t uid, int initialPid, int signal, int retries, 
                      << " in " << static_cast<int>(ms) << "ms";

        }



        if (!CgroupsAvailable()) {

            // nothing to do here, if cgroups isn't available

            return 0;

        }



        // 400 retries correspond to 2 secs max timeout

        int err = RemoveProcessGroup(cgroup, uid, initialPid, 400);