return 0;
}
-/*
- * Cleanup an exec after failures
+/** Cleans up an exec'd process on error
+ *
+ * This function is intended to be called at any point after a successful
+ * #fr_exec_wait_start_io call in order to release resources and cleanup
+ * zombie processes.
+ *
+ * @param[in] exec state to cleanup.
+ * @param[in] signal If non-zero, and we think the process is still
+ * running, send it a signal to cause it to exit.
+ * The PID reaper we insert here will cleanup its
+ * state so it doesn't become a zombie.
+ *
*/
-static void exec_cleanup(fr_exec_state_t *exec) {
+void fr_exec_cleanup(fr_exec_state_t *exec, int signal)
+{
request_t *request = exec->request;
- if (exec->pid) DEBUG3("Cleaning up exec state for pid %u", exec->pid);
+ if (exec->pid) RDEBUG3("Cleaning up exec state for pid %u", exec->pid);
+
+ /*
+ * There's still an EV_PROC event installed
+ * for the PID remove it (there's a destructor).
+ */
+ if (exec->ev_pid) {
+ talloc_const_free(exec->ev_pid);
+ fr_assert(!exec->ev_pid); /* Should be NULLified by destructor */
+ }
if (exec->stdout_fd >= 0) {
if (fr_event_fd_delete(request->el, exec->stdout_fd, FR_EVENT_FILTER_IO) < 0){
exec->stderr_fd = -1;
}
- if (exec->pid) {
- exec->pid = 0;
+ if (exec->pid >= 0) {
+ if (signal > 0) kill(exec->pid, signal);
+
if (fr_event_pid_reap(request->el, exec->pid) < 0) {
RPERROR("Failed setting up async PID reaper, PID %u may now be a zombie", exec->pid);
}
+ exec->pid = -1;
}
if (exec->ev) fr_event_timer_delete(&exec->ev);
} else {
REDEBUG("Timeout running program - killing it and failing the request");
}
- kill(exec->pid, SIGKILL);
exec->failed = true;
- exec_cleanup(exec);
+ fr_exec_cleanup(exec, SIGKILL);
unlang_interpret_mark_runnable(exec->request);
}
if (unlikely(!remaining)) {
REDEBUG("Too much output from program - killing it and failing the request");
- if (exec->pid > 0) kill(exec->pid, SIGKILL);
-
error:
exec->failed = true;
- exec_cleanup(exec);
+ fr_exec_cleanup(exec, SIGKILL);
break;
}
close(fd);
exec->stdout_fd = -1;
- if (exec->pid == 0) {
+ if (exec->pid < 0) {
/*
* Child has already exited - unlang can resume
*/
RPEDEBUG("Failed executing program");
fail:
/*
- * Not done in exec_cleanup as it's
+ * Not done in fr_exec_cleanup as it's
* usually the caller's responsibility.
*/
if (exec->stdin_fd >= 0) {
close(exec->stdin_fd);
exec->stdin_fd = -1;
}
- exec_cleanup(exec);
+ fr_exec_cleanup(exec, 0);
return -1;
}
* Tell the event loop that it needs to wait for this PID
*/
if (fr_event_pid_wait(ctx, request->el, &exec->ev_pid, exec->pid, exec_waitpid, exec) < 0) {
- exec->pid = 0;
+ exec->pid = -1;
RPEDEBUG("Failed adding watcher for child process");
fail_and_close:
/*
- * Avoid spurious errors in exec_cleanup
+ * Avoid spurious errors in fr_exec_cleanup
* when it tries to remove FDs from the
* event loop that were never added.
*/
request_t *request, char const *cmd, fr_pair_list_t *input_pairs,
bool exec_wait, bool shell_escape, fr_time_delta_t timeout) CC_HINT(nonnull (5, 6));
+void fr_exec_cleanup(fr_exec_state_t *exec, int signal);
+
int fr_exec_nowait(request_t *request, fr_value_box_list_t *vb_list, fr_pair_list_t *env_pairs);
int fr_exec_wait_start(pid_t *pid_p, int *stdin_fd, int *stdout_fd, int *stderr_fd,
#include <sys/wait.h>
#endif
-/*
- * Clean up everything except the waitpid handler.
- *
- * If there is a waitpid handler, then this cleanup function MUST
- * be called after setting the handler.
- */
-static void unlang_tmpl_exec_cleanup(request_t *request)
-{
- unlang_stack_t *stack = request->stack;
- unlang_stack_frame_t *frame = &stack->frame[stack->depth];
- unlang_frame_state_tmpl_t *state = talloc_get_type_abort(frame->state,
- unlang_frame_state_tmpl_t);
-
- if (state->exec.pid) RDEBUG3("Cleaning up exec state for pid %u", state->exec.pid);
-
- if (state->exec.stdout_fd >= 0) {
- if (fr_event_fd_delete(request->el, state->exec.stdout_fd, FR_EVENT_FILTER_IO) < 0) {
- RPERROR("Failed removing stdout handler");
- }
- close(state->exec.stdout_fd);
- state->exec.stdout_fd = -1;
- }
-
- if (state->exec.stderr_fd >= 0) {
- if (fr_event_fd_delete(request->el, state->exec.stderr_fd, FR_EVENT_FILTER_IO) < 0) {
- RPERROR("Failed removing stderr handler");
- }
- close(state->exec.stdout_fd);
- state->exec.stdout_fd = -1;
- }
-
- /*
- * It still hasn't exited. Tell the event loop that we
- * need to wait as long as necessary for the PID to exit,
- * and that we don't care about the exit status.
- */
- if (state->exec.pid) {
- (void) fr_event_pid_wait(request->el, request->el, NULL, state->exec.pid, NULL, NULL);
- state->exec.pid = 0;
- }
-
- if (state->exec.ev) fr_event_timer_delete(&state->exec.ev);
-}
-
/** Send a signal (usually stop) to a request
*
* This is typically called via an "async" action, i.e. an action
unlang_frame_state_tmpl_t *state = talloc_get_type_abort(frame->state,
unlang_frame_state_tmpl_t);
+ /*
+ * If we're cancelled, then kill any child processes
+ */
+ if (action == FR_SIGNAL_CANCEL) fr_exec_cleanup(&state->exec, SIGKILL);
+
if (!state->signal) return;
state->signal(request, state->rctx, action);
/*
- * If we're cancelled, then kill any child processes, and
- * ignore future signals.
+ * If we're cancelled then disable this signal handler.
+ * fr_exec_cleanup should handle being called spuriously.
*/
- if (action == FR_SIGNAL_CANCEL) {
- if (state->exec.pid > 0) kill(state->exec.pid, SIGKILL);
- state->exec.failed = true;
-
- unlang_tmpl_exec_cleanup(request);
- state->signal = NULL;
- }
+ if (action == FR_SIGNAL_CANCEL) state->signal = NULL;
}
/** Push a tmpl onto the stack for evaluation
unlang_frame_state_tmpl_t *state = talloc_get_type_abort(frame->state,
unlang_frame_state_tmpl_t);
- unlang_tmpl_exec_cleanup(request);
-
/*
* The exec failed for some internal reason. We don't
* care about output, and we don't care about the programs exit status.
goto resume;
}
- fr_assert(state->exec.pid == 0);
+ fr_assert(state->exec.pid < 0);
if (state->exec.status != 0) {
fr_assert(fr_dlist_empty(&state->box));
callback = pev->callback;
uctx = pev->uctx;
- pev->pid = 0; /* so we won't hit kevent again when it's freed */
+ pev->pid = -1; /* so we won't hit kevent again when it's freed */
/*
* Delete the event before calling it.