Revert "init: Fix a race condition in KillProcessGroup()"
This reverts commit 15e5ecdcd7ea69e63387346bd235fbeb3e3597df.
Reason for revert: breaks console support.
Bug: 213617178
Bug: 258754901
Change-Id: Iffe213e2cd295461a427621f2b84933f1bebd39f
diff --git a/init/service.cpp b/init/service.cpp
index 85ac2fc..c260c07 100644
--- a/init/service.cpp
+++ b/init/service.cpp
@@ -16,7 +16,6 @@
#include "service.h"
-#include <errno.h>
#include <fcntl.h>
#include <inttypes.h>
#include <linux/securebits.h>
@@ -533,6 +532,7 @@
if (!byte.ok()) {
LOG(ERROR) << name_ << ": failed to read from notification channel: " << byte.error();
}
+ fifo.Close();
if (!*byte) {
LOG(FATAL) << "Service '" << name_ << "' failed to start due to a fatal error";
_exit(EXIT_FAILURE);
@@ -556,12 +556,6 @@
// priority. Aborts on failure.
SetProcessAttributesAndCaps();
- // If SetProcessAttributes() called setsid(), report this to the parent.
- if (RequiresConsole(proc_attr_)) {
- fifo.Write(2);
- }
- fifo.Close();
-
if (!ExpandArgsAndExecv(args_, sigstop_)) {
PLOG(ERROR) << "cannot execv('" << args_[0]
<< "'). See the 'Debugging init' section of init's README.md for tips";
@@ -662,8 +656,11 @@
if (pid == 0) {
umask(077);
+ fifo.CloseWriteFd();
RunService(descriptors, std::move(fifo));
_exit(127);
+ } else {
+ fifo.CloseReadFd();
}
if (pid < 0) {
@@ -720,31 +717,6 @@
return Error() << "Sending cgroups activated notification failed: " << result.error();
}
- // Call setpgid() from the parent process to make sure that this call has
- // finished before the parent process calls kill(-pgid, ...).
- if (proc_attr_.console.empty()) {
- if (setpgid(pid, pid) < 0) {
- switch (errno) {
- case EACCES: // Child has already performed execve().
- case ESRCH: // Child process no longer exists.
- break;
- default:
- PLOG(ERROR) << "setpgid() from parent failed";
- }
- }
- } else {
- // The Read() call below will return an error if the child is killed.
- if (Result<uint8_t> result = fifo.Read(); !result.ok() || *result != 2) {
- if (!result.ok()) {
- return Error() << "Waiting for setsid() failed: " << result.error();
- } else {
- return Error() << "Waiting for setsid() failed: " << *result << " <> 2";
- }
- }
- }
-
- fifo.Close();
-
NotifyStateChange("running");
reboot_on_failure.Disable();
return {};
diff --git a/init/service.h b/init/service.h
index 2c2778d..b2c9909 100644
--- a/init/service.h
+++ b/init/service.h
@@ -155,7 +155,7 @@
void ResetFlagsForStart();
Result<void> CheckConsole();
void ConfigureMemcg();
- void RunService(const std::vector<Descriptor>& descriptors, InterprocessFifo fifo);
+ void RunService(const std::vector<Descriptor>& descriptors, InterprocessFifo cgroups_activated);
void SetMountNamespace();
static unsigned long next_start_order_;
static bool is_exec_service_running_;
diff --git a/init/service_utils.cpp b/init/service_utils.cpp
index 9585d05..a14969e 100644
--- a/init/service_utils.cpp
+++ b/init/service_utils.cpp
@@ -240,15 +240,11 @@
}
}
- if (RequiresConsole(attr)) {
+ if (!attr.console.empty()) {
setsid();
OpenConsole(attr.console);
} else {
- // Without PID namespaces, this call duplicates the setpgid() call from
- // the parent process. With PID namespaces, this setpgid() call sets the
- // process group ID for a child of the init process in the PID
- // namespace.
- if (setpgid(0, 0) == -1) {
+ if (setpgid(0, getpid()) == -1) {
return ErrnoError() << "setpgid failed";
}
SetupStdio(attr.stdio_to_kmsg);
diff --git a/init/service_utils.h b/init/service_utils.h
index c66f2b4..65a2012 100644
--- a/init/service_utils.h
+++ b/init/service_utils.h
@@ -89,11 +89,6 @@
int priority;
bool stdio_to_kmsg;
};
-
-inline bool RequiresConsole(const ProcessAttributes& attr) {
- return !attr.console.empty();
-}
-
Result<void> SetProcessAttributes(const ProcessAttributes& attr);
Result<void> WritePidToFiles(std::vector<std::string>* files);