~martanne/devel

allow subprocesses to shutdown v1 SUPERSEDED

Florian Fischer: 2
 move waiting and potentially killing a subprocess into a helper function
 check the life time of subprocesses before freeing vis

 4 files changed, 52 insertions(+), 20 deletions(-)
Hi Randy,
No the processes we leave behind are no zombies, they are still running and now effectively  daemonized.
See my inotifywait example above.

That's because the processes are not signaled when vis quits, except they have asked for receiving SIGHUP when the parent exits.

And invalidating the process handle and thus attempting to send them SIGTERM during the QUIT event does not work either, because the subprocesses are not handled after the QUIT event was emitted.
And this is what the patches change.
There is now one last round of checking each subprocess, detecting possible invalidated handles and sending SIGTERM.
This change allows plugin developers to shutdown their subprocesses on exit.

Hope this better explains what is going on.

Flo
Hi Karthin,
Next
Export patchset (mbox)
How do I use this?

Copy & paste the following snippet into your terminal to import this patchset into git:

curl -s https://lists.sr.ht/~martanne/devel/patches/55625/mbox | git am -3
Learn more about email & git

[PATCH 1/2] move waiting and potentially killing a subprocess into a helper function Export this patch

The separation between reading from a subprocess and handling its
life time will be useful for future changes.
---
 vis-subprocess.c | 54 ++++++++++++++++++++++++++++++------------------
 1 file changed, 34 insertions(+), 20 deletions(-)

diff --git a/vis-subprocess.c b/vis-subprocess.c
index 39282e47..3fe1dde7 100644
--- a/vis-subprocess.c
+++ b/vis-subprocess.c
@@ -176,6 +176,36 @@ static void read_and_fire(Vis* vis, int fd, const char *name, ResponseType rtype
	}
}

/**
 * Checks if a subprocess is dead or needs to be killed then raises an event
 * or kills it if necessary.
 * @param current the process to wait for or kill
 * @return true if the process is dead
 */
static int wait_or_kill_process(Vis *vis, Process *current) {
	int status;
	pid_t wpid = waitpid(current->pid, &status, WNOHANG);
	if (wpid == -1) {
		vis_message_show(vis, strerror(errno));
	} else if (wpid == current->pid) {
		goto just_destroy;
	} else if (!*(current->invalidator)) {
		goto kill_and_destroy;
	}
	return false;

kill_and_destroy:
		kill(current->pid, SIGTERM);
		waitpid(current->pid, &status, 0);
just_destroy:
		if (WIFSIGNALED(status)) {
			vis_lua_process_response(vis, current->name, NULL, WTERMSIG(status), SIGNAL);
		} else {
			vis_lua_process_response(vis, current->name, NULL, WEXITSTATUS(status), EXIT);
		}
	return true;
}

/**
 * Checks if `readfds` contains file descriptors of subprocesses from
 * the pool. If so, it reads their data and fires corresponding events.
@@ -192,27 +222,11 @@ void vis_process_tick(Vis *vis, fd_set *readfds) {
		if (current->errfd != -1 && FD_ISSET(current->errfd, readfds)) {
			read_and_fire(vis, current->errfd, current->name, STDERR);
		}
		int status;
		pid_t wpid = waitpid(current->pid, &status, WNOHANG);
		if (wpid == -1)	{
			vis_message_show(vis, strerror(errno));
		} else if (wpid == current->pid) {
			goto just_destroy;
		} else if (!*(current->invalidator)) {
			goto kill_and_destroy;
		}
		pointer = &current->next;
		continue;
kill_and_destroy:
		kill(current->pid, SIGTERM);
		waitpid(current->pid, &status, 0);
just_destroy:
		if (WIFSIGNALED(status)) {
			vis_lua_process_response(vis, current->name, NULL, WTERMSIG(status), SIGNAL);
		if (!wait_or_kill_process(vis, current)) {
			pointer = &current->next;
		} else {
			vis_lua_process_response(vis, current->name, NULL, WEXITSTATUS(status), EXIT);
			/* update our iteration pointer */
			*pointer = destroy_process(current);
		}
		/* update our iteration pointer */
		*pointer = destroy_process(current);
	}
}
-- 
2.47.0

[PATCH 2/2] check the life time of subprocesses before freeing vis Export this patch

Currently there is now way for long running subprocesses like language
servers to gracefully shutdown.
When reacting to the QUIT event and invalidating the process handle
the subprocess will never be killed and destroyed because the
subprocesses are only checked during vis_run.

Collecting and killing subprocesses with invalid handles after the
QUIT event allows graceful shutdown.
---
 vis-subprocess.c | 16 ++++++++++++++++
 vis-subprocess.h |  1 +
 vis.c            |  1 +
 3 files changed, 18 insertions(+)

diff --git a/vis-subprocess.c b/vis-subprocess.c
index 3fe1dde7..beac9998 100644
--- a/vis-subprocess.c
+++ b/vis-subprocess.c
@@ -230,3 +230,19 @@ void vis_process_tick(Vis *vis, fd_set *readfds) {
		}
	}
}

/**
 * Checks if each subprocess from the pool is dead or needs to be
 * killed then raises an event or kills it if necessary.
 */
void vis_process_waitall(Vis *vis) {
	for (Process **pointer = &process_pool; *pointer; ) {
		Process *current = *pointer;
		if (!wait_or_kill_process(vis, current)) {
			pointer = &current->next;
		} else {
			/* update our iteration pointer */
			*pointer = destroy_process(current);
		}
	}
}
diff --git a/vis-subprocess.h b/vis-subprocess.h
index 2e4c2220..79a043e9 100644
--- a/vis-subprocess.h
+++ b/vis-subprocess.h
@@ -27,4 +27,5 @@ Process *vis_process_communicate(Vis *, const char *command, const char *name,
                                 Invalidator **invalidator);
int vis_process_before_tick(fd_set *);
void vis_process_tick(Vis *, fd_set *);
void vis_process_waitall(Vis *);
#endif
diff --git a/vis.c b/vis.c
index 10c3df57..d1c0dab0 100644
--- a/vis.c
+++ b/vis.c
@@ -623,6 +623,7 @@ void vis_free(Vis *vis) {
	while (vis->windows)
		vis_window_close(vis->windows);
	vis_event_emit(vis, VIS_EVENT_QUIT);
	vis_process_waitall(vis);
	file_free(vis, vis->command_file);
	file_free(vis, vis->search_file);
	file_free(vis, vis->error_file);
-- 
2.47.0