[systemd-commits] stable Branch 'v208-stable' - 7 commits - src/core src/login src/shared
Lukas Nykryn
lnykryn at kemper.freedesktop.org
Fri Mar 14 09:56:33 PDT 2014
src/core/manager.c | 201 +++++++++++++++++++++++++--------------------
src/core/manager.h | 9 +-
src/core/service.c | 14 +++
src/core/unit.c | 57 +++++++-----
src/login/logind-session.c | 4
src/shared/util.c | 25 +++++
src/shared/util.h | 1
7 files changed, 197 insertions(+), 114 deletions(-)
New commits:
commit 4372855b7871dd4f639d0e5fca86eff0d38aeb18
Author: Yuxuan Shui <yshuiv7 at gmail.com>
Date: Sat Feb 15 13:20:55 2014 +0800
core: check for return value from get_process_state
Fix for commit e10c9985bb.
diff --git a/src/core/service.c b/src/core/service.c
index d9bc021..c8dbbef 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1429,11 +1429,17 @@ static int service_load_pid_file(Service *s, bool may_warn) {
return -ESRCH;
}
- if (get_process_state(pid) == 'Z') {
+ r = get_process_state(pid);
+ if (r < 0) {
+ if (may_warn)
+ log_info_unit(UNIT(s)->id, "Failed to read /proc/%d/stat: %s",
+ pid, strerror(-r));
+ return r;
+ } else if (r == 'Z') {
if (may_warn)
log_info_unit(UNIT(s)->id,
- "PID "PID_FMT" read from file %s is a zombie.",
- pid, s->pid_file);
+ "PID %lu read from file %s is a zombie.",
+ (unsigned long) pid, s->pid_file);
return -ESRCH;
}
commit a4157894c590dc1a976449271be4b6faecf22a28
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date: Fri Feb 14 23:04:50 2014 -0500
Fix prototype of get_process_state
diff --git a/src/shared/util.h b/src/shared/util.h
index ca38336..02621a7 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -216,7 +216,7 @@ char *file_in_same_dir(const char *path, const char *filename);
int rmdir_parents(const char *path, const char *stop);
-char get_process_state(pid_t pid);
+int get_process_state(pid_t pid);
int get_process_comm(pid_t pid, char **name);
int get_process_cmdline(pid_t pid, size_t max_length, bool comm_fallback, char **line);
int get_process_exe(pid_t pid, char **name);
commit 0ab7a0e4655fd1108fa3abc994f816a22073abc6
Author: Yuxuan Shui <yshuiv7 at gmail.com>
Date: Sat Feb 15 02:38:50 2014 +0800
core: fix detection of dead processes
Commit 5ba6985b moves the UNIT_VTABLE(u)->sigchld_event before systemd
actually reaps the zombie. Which leads to service_load_pid_file accepting
zombie as a valid pid.
This fixes timeouts like:
[ 2746.602243] systemd[1]: chronyd.service stop-sigterm timed out. Killing.
[ 2836.852545] systemd[1]: chronyd.service still around after SIGKILL. Ignoring.
[ 2927.102187] systemd[1]: chronyd.service stop-final-sigterm timed out. Killing.
[ 3017.352560] systemd[1]: chronyd.service still around after final SIGKILL. Entering failed mode.
diff --git a/src/core/service.c b/src/core/service.c
index 41e5cb5..d9bc021 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1429,6 +1429,14 @@ static int service_load_pid_file(Service *s, bool may_warn) {
return -ESRCH;
}
+ if (get_process_state(pid) == 'Z') {
+ if (may_warn)
+ log_info_unit(UNIT(s)->id,
+ "PID "PID_FMT" read from file %s is a zombie.",
+ pid, s->pid_file);
+ return -ESRCH;
+ }
+
if (s->main_pid_known) {
if (pid == s->main_pid)
return 0;
diff --git a/src/shared/util.c b/src/shared/util.c
index e754747..1329854 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -558,6 +558,31 @@ char *truncate_nl(char *s) {
return s;
}
+int get_process_state(pid_t pid) {
+ const char *p;
+ char state;
+ int r;
+ _cleanup_free_ char *line = NULL;
+
+ assert(pid >= 0);
+
+ p = procfs_file_alloca(pid, "stat");
+ r = read_one_line_file(p, &line);
+ if (r < 0)
+ return r;
+
+ p = strrchr(line, ')');
+ if (!p)
+ return -EIO;
+
+ p++;
+
+ if (sscanf(p, " %c", &state) != 1)
+ return -EIO;
+
+ return (unsigned char) state;
+}
+
int get_process_comm(pid_t pid, char **name) {
const char *p;
diff --git a/src/shared/util.h b/src/shared/util.h
index bdbdca3..ca38336 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -216,6 +216,7 @@ char *file_in_same_dir(const char *path, const char *filename);
int rmdir_parents(const char *path, const char *stop);
+char get_process_state(pid_t pid);
int get_process_comm(pid_t pid, char **name);
int get_process_cmdline(pid_t pid, size_t max_length, bool comm_fallback, char **line);
int get_process_exe(pid_t pid, char **name);
commit 495d57fb7487f29965a712acdc21f96273aa5c1d
Author: Michal Sekletar <msekleta at redhat.com>
Date: Thu Mar 13 11:18:27 2014 +0100
logind: add forgotten return statement
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 730f11d..ece222a 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -782,6 +782,8 @@ void session_release(Session *s) {
goto out;
}
+ return;
+
out:
if (s->timer_fd >= 0) {
close_nointr(s->timer_fd);
commit 5cadb261e83dbc70fd42095e78c5862dac09dc65
Author: Michal Sekletar <msekleta at redhat.com>
Date: Thu Mar 13 11:16:36 2014 +0100
logind: uninitialized timer_fd is set to -1
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index b0e4bf6..730f11d 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -657,7 +657,7 @@ static int session_unlink_x11_socket(Session *s) {
static void session_close_timer_fd(Session *s) {
assert(s);
- if (s->timer_fd <= 0)
+ if (s->timer_fd < 0)
return;
hashmap_remove(s->manager->timer_fds, INT_TO_PTR(s->timer_fd + 1));
commit ba421b96a473a4e608c2b1d01d3bb3dfe36c23a6
Author: Lennart Poettering <lennart at poettering.net>
Date: Thu Mar 6 02:19:42 2014 +0100
core: correctly unregister PIDs from PID hashtables
Conflicts:
src/core/unit.c
diff --git a/src/core/unit.c b/src/core/unit.c
index dc88862..9a7720d 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -1666,11 +1666,11 @@ int unit_watch_pid(Unit *u, pid_t pid) {
/* Watch a specific PID. We only support one or two units
* watching each PID for now, not more. */
- r = hashmap_ensure_allocated(&u->manager->watch_pids1, trivial_hash_func, trivial_compare_func);
+ r = set_ensure_allocated(&u->pids, trivial_hash_func, trivial_compare_func);
if (r < 0)
return r;
- r = set_ensure_allocated(&u->pids, trivial_hash_func, trivial_compare_func);
+ r = hashmap_ensure_allocated(&u->manager->watch_pids1, trivial_hash_func, trivial_compare_func);
if (r < 0)
return r;
@@ -1699,7 +1699,17 @@ void unit_unwatch_pid(Unit *u, pid_t pid) {
set_remove(u->pids, LONG_TO_PTR(pid));
}
-static int watch_pids_in_path(Unit *u, const char *path) {
+void unit_unwatch_all_pids(Unit *u) {
+ assert(u);
+
+ while (!set_isempty(u->pids))
+ unit_unwatch_pid(u, PTR_TO_LONG(set_first(u->pids)));
+
+ set_free(u->pids);
+ u->pids = NULL;
+}
+
+static int unit_watch_pids_in_path(Unit *u, const char *path) {
_cleanup_closedir_ DIR *d = NULL;
_cleanup_fclose_ FILE *f = NULL;
int ret = 0, r;
@@ -1737,7 +1747,7 @@ static int watch_pids_in_path(Unit *u, const char *path) {
if (!p)
return -ENOMEM;
- r = watch_pids_in_path(u, p);
+ r = unit_watch_pids_in_path(u, p);
if (r < 0 && ret >= 0)
ret = r;
}
@@ -1754,27 +1764,12 @@ static int watch_pids_in_path(Unit *u, const char *path) {
int unit_watch_all_pids(Unit *u) {
assert(u);
- if (!u->cgroup_path)
- return -ENOENT;
-
/* Adds all PIDs from our cgroup to the set of PIDs we watch */
- return watch_pids_in_path(u, u->cgroup_path);
-}
-
-void unit_unwatch_all_pids(Unit *u) {
- Iterator i;
- void *e;
-
- assert(u);
-
- SET_FOREACH(e, u->pids, i) {
- hashmap_remove_value(u->manager->watch_pids1, e, u);
- hashmap_remove_value(u->manager->watch_pids2, e, u);
- }
+ if (!u->cgroup_path)
+ return -ENOENT;
- set_free(u->pids);
- u->pids = NULL;
+ return unit_watch_pids_in_path(u, u->cgroup_path);
}
void unit_tidy_watch_pids(Unit *u, pid_t except1, pid_t except2) {
@@ -1792,7 +1787,7 @@ void unit_tidy_watch_pids(Unit *u, pid_t except1, pid_t except2) {
continue;
if (kill(pid, 0) < 0 && errno == ESRCH)
- set_remove(u->pids, e);
+ unit_unwatch_pid(u, pid);
}
}
commit 39c82bc4ed40b4ce20f85559afc67ad16459cef5
Author: Lennart Poettering <lennart at poettering.net>
Date: Fri Feb 7 11:58:25 2014 +0100
core: allow PIDs to be watched by two units at the same time
In some cases it is interesting to map a PID to two units at the same
time. For example, when a user logs in via a getty, which is reexeced to
/sbin/login that binary will be explicitly referenced as main pid of the
getty service, as well as implicitly referenced as part of the session
scope.
Conflicts:
src/core/manager.c
src/core/manager.h
diff --git a/src/core/manager.c b/src/core/manager.c
index e7b5234..58c17ab 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -525,7 +525,10 @@ int manager_new(SystemdRunningAs running_as, bool reexecuting, Manager **_m) {
if (!(m->jobs = hashmap_new(trivial_hash_func, trivial_compare_func)))
goto fail;
- if (!(m->watch_pids = hashmap_new(trivial_hash_func, trivial_compare_func)))
+ if (!(m->watch_pids1 = hashmap_new(trivial_hash_func, trivial_compare_func)))
+ goto fail;
+
+ if (!(m->watch_pids2 = hashmap_new(trivial_hash_func, trivial_compare_func)))
goto fail;
m->cgroup_unit = hashmap_new(string_hash_func, string_compare_func);
@@ -740,7 +743,8 @@ void manager_free(Manager *m) {
hashmap_free(m->units);
hashmap_free(m->jobs);
- hashmap_free(m->watch_pids);
+ hashmap_free(m->watch_pids1);
+ hashmap_free(m->watch_pids2);
hashmap_free(m->watch_bus);
if (m->epoll_fd >= 0)
@@ -1247,6 +1251,26 @@ unsigned manager_dispatch_dbus_queue(Manager *m) {
return n;
}
+static void manager_invoke_notify_message(Manager *m, Unit *u, pid_t pid, char *buf, size_t n) {
+ _cleanup_strv_free_ char **tags = NULL;
+
+ assert(m);
+ assert(u);
+ assert(buf);
+ assert(n > 0);
+
+ tags = strv_split(buf, "\n\r");
+ if (!tags) {
+ log_oom();
+ return;
+ }
+
+ log_debug_unit(u->id, "Got notification message for unit %s", u->id);
+
+ if (UNIT_VTABLE(u)->notify_message)
+ UNIT_VTABLE(u)->notify_message(u, pid, tags);
+}
+
static int manager_process_notify_fd(Manager *m) {
ssize_t n;
@@ -1259,6 +1283,8 @@ static int manager_process_notify_fd(Manager *m) {
.iov_len = sizeof(buf)-1,
};
+ bool found = false;
+
union {
struct cmsghdr cmsghdr;
uint8_t buf[CMSG_SPACE(sizeof(struct ucred))];
@@ -1272,7 +1298,6 @@ static int manager_process_notify_fd(Manager *m) {
};
struct ucred *ucred;
Unit *u;
- _cleanup_strv_free_ char **tags = NULL;
n = recvmsg(m->notify_watch.fd, &msghdr, MSG_DONTWAIT);
if (n <= 0) {
@@ -1295,105 +1320,105 @@ static int manager_process_notify_fd(Manager *m) {
ucred = (struct ucred*) CMSG_DATA(&control.cmsghdr);
- u = hashmap_get(m->watch_pids, LONG_TO_PTR(ucred->pid));
- if (!u) {
- u = manager_get_unit_by_pid(m, ucred->pid);
- if (!u) {
- log_warning("Cannot find unit for notify message of PID %lu.", (unsigned long) ucred->pid);
- continue;
- }
- }
-
assert((size_t) n < sizeof(buf));
buf[n] = 0;
- tags = strv_split(buf, "\n\r");
- if (!tags)
- return log_oom();
-
- log_debug_unit(u->id, "Got notification message for unit %s", u->id);
-
- if (UNIT_VTABLE(u)->notify_message)
- UNIT_VTABLE(u)->notify_message(u, ucred->pid, tags);
- }
-
- return 0;
-}
-
-static int manager_dispatch_sigchld(Manager *m) {
- assert(m);
-
- for (;;) {
- siginfo_t si = {};
- Unit *u;
- int r;
-
- /* First we call waitd() for a PID and do not reap the
- * zombie. That way we can still access /proc/$PID for
- * it while it is a zombie. */
- if (waitid(P_ALL, 0, &si, WEXITED|WNOHANG|WNOWAIT) < 0) {
-
- if (errno == ECHILD)
- break;
- if (errno == EINTR)
- continue;
-
- return -errno;
+ u = manager_get_unit_by_pid(m, ucred->pid);
+ if (u) {
+ manager_invoke_notify_message(m, u, ucred->pid, buf, n);
+ found = true;
}
- if (si.si_pid <= 0)
- break;
-
- if (si.si_code == CLD_EXITED || si.si_code == CLD_KILLED || si.si_code == CLD_DUMPED) {
- _cleanup_free_ char *name = NULL;
-
- get_process_comm(si.si_pid, &name);
- log_debug("Got SIGCHLD for process %lu (%s)", (unsigned long) si.si_pid, strna(name));
+ u = hashmap_get(m->watch_pids1, LONG_TO_PTR(ucred->pid));
+ if (u) {
+ manager_invoke_notify_message(m, u, ucred->pid, buf, n);
+ found = true;
}
- /* Let's flush any message the dying child might still
- * have queued for us. This ensures that the process
- * still exists in /proc so that we can figure out
- * which cgroup and hence unit it belongs to. */
- r = manager_process_notify_fd(m);
- if (r < 0)
- return r;
-
- /* And now figure out the unit this belongs to */
- u = hashmap_get(m->watch_pids, LONG_TO_PTR(si.si_pid));
- if (!u)
- u = manager_get_unit_by_pid(m, si.si_pid);
-
- /* And now, we actually reap the zombie. */
- if (waitid(P_PID, si.si_pid, &si, WEXITED) < 0) {
- if (errno == EINTR)
- continue;
-
- return -errno;
+ u = hashmap_get(m->watch_pids2, LONG_TO_PTR(ucred->pid));
+ if (u) {
+ manager_invoke_notify_message(m, u, ucred->pid, buf, n);
+ found = true;
}
- if (si.si_code != CLD_EXITED && si.si_code != CLD_KILLED && si.si_code != CLD_DUMPED)
- continue;
+ if (!found)
+ log_warning("Cannot find unit for notify message of PID %lu.",(long unsigned) ucred->pid);
+ }
- log_debug("Child %lu died (code=%s, status=%i/%s)",
- (long unsigned) si.si_pid,
- sigchld_code_to_string(si.si_code),
- si.si_status,
- strna(si.si_code == CLD_EXITED
- ? exit_status_to_string(si.si_status, EXIT_STATUS_FULL)
- : signal_to_string(si.si_status)));
+ return 0;
+}
- if (!u)
- continue;
+static void invoke_sigchld_event(Manager *m, Unit *u, siginfo_t *si) {
+ assert(m);
+ assert(u);
+ assert(si);
- log_debug_unit(u->id,
- "Child %lu belongs to %s", (long unsigned) si.si_pid, u->id);
+ log_debug_unit(u->id, "Child %lu belongs to %s",(long unsigned) si->si_pid, u->id);
- unit_unwatch_pid(u, si.si_pid);
- UNIT_VTABLE(u)->sigchld_event(u, si.si_pid, si.si_code, si.si_status);
- }
+ unit_unwatch_pid(u, si->si_pid);
+ UNIT_VTABLE(u)->sigchld_event(u, si->si_pid, si->si_code, si->si_status);
+}
- return 0;
+static int manager_dispatch_sigchld(Manager *m) {
+ assert(m);
+
+ for (;;) {
+ siginfo_t si = {};
+
+ /* First we call waitd() for a PID and do not reap the
+ * zombie. That way we can still access /proc/$PID for
+ * it while it is a zombie. */
+ if (waitid(P_ALL, 0, &si, WEXITED|WNOHANG|WNOWAIT) < 0) {
+
+ if (errno == ECHILD)
+ break;
+
+ if (errno == EINTR)
+ continue;
+
+ return -errno;
+ }
+
+ if (si.si_pid <= 0)
+ break;
+
+ if (si.si_code == CLD_EXITED || si.si_code == CLD_KILLED || si.si_code == CLD_DUMPED) {
+ _cleanup_free_ char *name = NULL;
+ Unit *u;
+
+ get_process_comm(si.si_pid, &name);
+
+ log_debug("Child %lu (%s) died (code=%s, status=%i/%s)",
+ (long unsigned) si.si_pid, strna(name),
+ sigchld_code_to_string(si.si_code),
+ si.si_status,
+ strna(si.si_code == CLD_EXITED
+ ? exit_status_to_string(si.si_status, EXIT_STATUS_FULL)
+ : signal_to_string(si.si_status)));
+
+ /* And now figure out the unit this belongs
+ * to, it might be multiple... */
+ u = manager_get_unit_by_pid(m, si.si_pid);
+ if (u)
+ invoke_sigchld_event(m, u, &si);
+ u = hashmap_get(m->watch_pids1, LONG_TO_PTR(si.si_pid));
+ if (u)
+ invoke_sigchld_event(m, u, &si);
+ u = hashmap_get(m->watch_pids2, LONG_TO_PTR(si.si_pid));
+ if (u)
+ invoke_sigchld_event(m, u, &si);
+ }
+
+ /* And now, we actually reap the zombie. */
+ if (waitid(P_PID, si.si_pid, &si, WEXITED) < 0) {
+ if (errno == EINTR)
+ continue;
+
+ return -errno;
+ }
+ }
+
+ return 0;
}
static int manager_start_target(Manager *m, const char *name, JobMode mode) {
diff --git a/src/core/manager.h b/src/core/manager.h
index ee42c5e..0133ea5 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -125,7 +125,14 @@ struct Manager {
/* Units that should be realized */
LIST_HEAD(Unit, cgroup_queue);
- Hashmap *watch_pids; /* pid => Unit object n:1 */
+ /* We use two hash tables here, since the same PID might be
+ * watched by two different units: once the unit that forked
+ * it off, and possibly a different unit to which it was
+ * joined as cgroup member. Since we know that it is either
+ * one or two units for each PID we just use to hashmaps
+ * here. */
+ Hashmap *watch_pids1; /* pid => Unit object n:1 */
+ Hashmap *watch_pids2; /* pid => Unit object n:1 */
char *notify_socket;
diff --git a/src/core/unit.c b/src/core/unit.c
index 57a406d..dc88862 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -1663,16 +1663,27 @@ int unit_watch_pid(Unit *u, pid_t pid) {
assert(u);
assert(pid >= 1);
+ /* Watch a specific PID. We only support one or two units
+ * watching each PID for now, not more. */
+
+ r = hashmap_ensure_allocated(&u->manager->watch_pids1, trivial_hash_func, trivial_compare_func);
+ if (r < 0)
+ return r;
+
r = set_ensure_allocated(&u->pids, trivial_hash_func, trivial_compare_func);
if (r < 0)
return r;
- /* Watch a specific PID. We only support one unit watching
- * each PID for now. */
+ r = hashmap_put(u->manager->watch_pids1, LONG_TO_PTR(pid), u);
+ if (r == -EEXIST) {
+ r = hashmap_ensure_allocated(&u->manager->watch_pids2, trivial_hash_func, trivial_compare_func);
+ if (r < 0)
+ return r;
- r = set_put(u->pids, LONG_TO_PTR(pid));
+ r = hashmap_put(u->manager->watch_pids2, LONG_TO_PTR(pid), u);
+ }
- q = hashmap_put(u->manager->watch_pids, LONG_TO_PTR(pid), u);
+ q = set_put(u->pids, LONG_TO_PTR(pid));
if (q < 0)
return q;
@@ -1683,7 +1694,8 @@ void unit_unwatch_pid(Unit *u, pid_t pid) {
assert(u);
assert(pid >= 1);
- hashmap_remove_value(u->manager->watch_pids, LONG_TO_PTR(pid), u);
+ hashmap_remove_value(u->manager->watch_pids1, LONG_TO_PTR(pid), u);
+ hashmap_remove_value(u->manager->watch_pids2, LONG_TO_PTR(pid), u);
set_remove(u->pids, LONG_TO_PTR(pid));
}
@@ -1756,8 +1768,10 @@ void unit_unwatch_all_pids(Unit *u) {
assert(u);
- SET_FOREACH(e, u->pids, i)
- hashmap_remove_value(u->manager->watch_pids, e, u);
+ SET_FOREACH(e, u->pids, i) {
+ hashmap_remove_value(u->manager->watch_pids1, e, u);
+ hashmap_remove_value(u->manager->watch_pids2, e, u);
+ }
set_free(u->pids);
u->pids = NULL;
More information about the systemd-commits
mailing list