[DTrace-devel] [PATCH 1/2] Revert "Exit properly when all dtrace -c processes end, even if waitfd() failed."
Nick Alcock
nick.alcock at oracle.com
Fri Oct 20 16:58:56 UTC 2023
This reverts commit e856890a68c5b2461f30499295f4f8583b8efed4.
We will no longer need it when waitfd is gone.
This also removes a whole pile of complexity where we might sometimes have
to synchronously check to see if the process has exited, simply because
we couldn't rely on waitfd() working everywhere.
Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
---
libdtrace/dt_consume.c | 6 +--
libdtrace/dt_proc.c | 95 ++++--------------------------------------
libdtrace/dt_proc.h | 2 -
3 files changed, 11 insertions(+), 92 deletions(-)
diff --git a/libdtrace/dt_consume.c b/libdtrace/dt_consume.c
index a67c0f7bf84c4..2d59dd5da7d40 100644
--- a/libdtrace/dt_consume.c
+++ b/libdtrace/dt_consume.c
@@ -2939,12 +2939,10 @@ dt_consume_proc_exits(dtrace_hdl_t *dtp)
dt_proc_notify_t *dprn;
/*
- * Make sure that any synchronous notifications of process exit are
- * received. Regardless of why we awaken, iterate over any pending
- * notifications and process them.
+ * Iterate over any pending notifications that may have accumulated
+ * while we were asleep, and process them.
*/
pthread_mutex_lock(&dph->dph_lock);
- dt_proc_enqueue_exits(dtp);
while ((dprn = dph->dph_notify) != NULL) {
if (dtp->dt_prochdlr != NULL) {
diff --git a/libdtrace/dt_proc.c b/libdtrace/dt_proc.c
index f531329db2cd7..737abd78f62d0 100644
--- a/libdtrace/dt_proc.c
+++ b/libdtrace/dt_proc.c
@@ -129,7 +129,7 @@ dt_unwinder_pad(struct ps_prochandle *unused)
static void
dt_proc_notify(dtrace_hdl_t *dtp, dt_proc_hash_t *dph, dt_proc_t *dpr,
- pid_t pid, const char *msg, int lock, int broadcast)
+ pid_t pid, const char *msg)
{
dt_proc_notify_t *dprn = dt_alloc(dtp, sizeof(dt_proc_notify_t));
@@ -144,82 +144,14 @@ dt_proc_notify(dtrace_hdl_t *dtp, dt_proc_hash_t *dph, dt_proc_t *dpr,
strlcpy(dprn->dprn_errmsg, msg,
sizeof(dprn->dprn_errmsg));
- if (lock)
- pthread_mutex_lock(&dph->dph_lock);
+ pthread_mutex_lock(&dph->dph_lock);
dprn->dprn_next = dph->dph_notify;
dprn->dprn_pid = pid;
dph->dph_notify = dprn;
- if (broadcast)
- eventfd_write(dtp->dt_proc_fd, 1);
- if (lock)
- pthread_mutex_unlock(&dph->dph_lock);
- }
-}
-
-/*
- * Wait for all processes in the dtp's hash table that are marked as dpr_created
- * but have no controlling thread. (These processes are counted in the dph, so
- * in the common case that there are none, this function can exit early and do
- * nothing.)
- *
- * Must be called under the dph_lock by the function that scans the notification
- * table, since no notifications are sent, only enqueued.
- */
-void
-dt_proc_enqueue_exits(dtrace_hdl_t *dtp)
-{
- dt_proc_hash_t *dph = dtp->dt_procs;
- dt_proc_t *dpr;
-
- if (!dph->dph_noninvasive_created)
- return;
-
- assert(MUTEX_HELD(&dph->dph_lock));
- /*
- * We can cheat here and save time by not taking out the
- * dt_proc_lock, because in the case we are interested in there is
- * no control thread to lock against, and in the only other case where
- * dpr_done can end up set and dpr_tid be NULL, the control thread has
- * died and the process has already been waited for.
- *
- * We also know that the process cannot be ptrace()d in this case, so
- * there is no danger of a return indicating a trace stop.
- */
- for (dpr = dt_list_next(&dph->dph_lrulist);
- dpr != NULL; dpr = dt_list_next(dpr)) {
- if (dpr->dpr_tid == 0 && dpr->dpr_done && dpr->dpr_proc) {
- int exited = 0;
- pid_t pid = Pgetpid(dpr->dpr_proc);
- siginfo_t info;
-
- info.si_pid = 0;
- /*
- * We treat -ECHILD like a process exit, and consider
- * other errors a real problem.
- */
- if (waitid(P_PID, pid, &info, WNOHANG | WEXITED) < 0) {
- if (errno != -ECHILD)
- dt_dprintf("Error waitid()ing for child "
- "%i: %s\n", Pgetpid(dpr->dpr_proc),
- strerror(errno));
- else
- exited = 1;
- }
-
- if (info.si_pid != 0)
- exited = 1;
-
- if (exited) {
- dt_proc_notify(dtp, dph, dpr, pid, NULL,
- B_FALSE, B_FALSE);
- Prelease(dpr->dpr_proc,
- dpr->dpr_created ? PS_RELEASE_KILL :
- PS_RELEASE_NORMAL);
- dph->dph_noninvasive_created--;
- }
- }
+ eventfd_write(dtp->dt_proc_fd, 1);
+ pthread_mutex_unlock(&dph->dph_lock);
}
}
@@ -417,7 +349,7 @@ dt_proc_scan(dtrace_hdl_t *dtp, dt_proc_t *dpr)
Pupdate_syms(dpr->dpr_proc);
if (dt_pid_create_probes_module(dtp, dpr) != 0)
dt_proc_notify(dtp, dtp->dt_procs, dpr, dpr->dpr_pid,
- dpr->dpr_errmsg, B_TRUE, B_TRUE);
+ dpr->dpr_errmsg);
}
/*
@@ -926,9 +858,7 @@ dt_proc_control(void *arg)
/*
* If this was a noninvasive grab, quietly exit without calling
* the cleanup handlers: the process is running, but does not
- * need a monitoring thread. Process termination detection is
- * handled in the parent process, via the subreaper already set
- * up.
+ * need a monitoring thread.
*/
if (noninvasive && !Ptraceable(dpr->dpr_proc)) {
dt_dprintf("%i: noninvasive grab, control thread "
@@ -956,9 +886,6 @@ dt_proc_control(void *arg)
* Pcreate()d it, dpr_created is still set, so it will still get
* killed on dtrace exit. If even that fails, there's nothing
* we can do but hope.
- *
- * The dt_consume_proc_exits() function, called by
- * dtrace_consume(), checks for termination of such processes * (since nothing else will).
*/
Prelease(dpr->dpr_proc, PS_RELEASE_NORMAL);
if ((dpr->dpr_proc = Pgrab(dpr->dpr_pid, 2, 0,
@@ -967,7 +894,6 @@ dt_proc_control(void *arg)
(long)dpr->dpr_pid, strerror(err));
}
- dtp->dt_procs->dph_noninvasive_created++;
pthread_exit(NULL);
}
@@ -1408,8 +1334,7 @@ dt_proc_control_cleanup(void *arg)
*/
if (!suiciding && dpr->dpr_notifiable)
dt_proc_notify(dpr->dpr_hdl, dpr->dpr_hdl->dt_procs,
- dpr->dpr_proc ? dpr : NULL, dpr->dpr_pid, NULL,
- B_TRUE, B_TRUE);
+ dpr->dpr_proc ? dpr : NULL, dpr->dpr_pid, NULL);
/*
* Signal on the cv, waking up any waiters once the lock is released.
@@ -1619,14 +1544,12 @@ dt_proc_destroy(dtrace_hdl_t *dtp, dt_proc_t *dpr)
*/
proxy_quit(dpr, 0);
dpr->dpr_lock_holder = pthread_self();
- } else if (dpr->dpr_proc) {
+ } else {
/*
* The process control thread is already dead, but try to clean
* the process up anyway, just in case it survived to this
* point. This can happen e.g. if the process was noninvasively
* grabbed and its control thread suicided.)
- *
- * It might be cleaned up already by dt_consume_proc_exits().
*/
Prelease(dpr->dpr_proc, dpr->dpr_created ? PS_RELEASE_KILL :
PS_RELEASE_NORMAL);
@@ -1768,7 +1691,7 @@ dt_proc_create(dtrace_hdl_t *dtp, const char *file, char *const *argv,
}
/*
- * Newly-created processes should be invasively grabbed.
+ * Newly-created processes must be invasively grabbed.
*/
if (flags & DTRACE_PROC_SHORTLIVED)
flags &= ~DTRACE_PROC_SHORTLIVED;
diff --git a/libdtrace/dt_proc.h b/libdtrace/dt_proc.h
index 77e65145df7a1..a08922bc68483 100644
--- a/libdtrace/dt_proc.h
+++ b/libdtrace/dt_proc.h
@@ -138,7 +138,6 @@ typedef struct dt_proc_hash {
uint_t dph_lrulim; /* limit on number of procs to hold */
uint_t dph_lrucnt; /* count of cached process handles */
uint_t dph_hashlen; /* size of hash chains array */
- uint_t dph_noninvasive_created; /* count of noninvasive -c procs */
dt_proc_t *dph_hash[1]; /* hash chains array */
} dt_proc_hash_t;
@@ -147,7 +146,6 @@ extern void dt_proc_release_unlock(dtrace_hdl_t *, pid_t);
extern void dt_proc_lock(dt_proc_t *dpr);
extern void dt_proc_unlock(dt_proc_t *dpr);
extern dt_proc_t *dt_proc_lookup(dtrace_hdl_t *, pid_t);
-extern void dt_proc_enqueue_exits(dtrace_hdl_t *dtp);
/*
* Proxies for operations in libproc, respecting the execve-retry protocol.
base-commit: cb3a9e81efd9d4fbc413b9427f86af079b5faab6
--
2.42.0.271.g85384428f1
More information about the DTrace-devel
mailing list