test: one Xvfb launcher and one daemon for the live tests

The XIM test carried its own Xvfb and daemon spawner, its own child
struct, log files and process-group teardown, and the GTK test a second
Xvfb launcher, because live.c's startdaemon always unset DISPLAY.  Live
now carries the display its own startxvfb reports, startdaemon passes it
on, and both tests use the shared pair: 300 lines fewer, one place that
knows how a child is started, watched, and stopped.

tests/gtk_live_test.c

@@ -437,73 +437,6 @@ runquery(char *module, char *cache)
 	return 1;
 }
 
-static int
-startxvfb(pid_t *pidp, char *display, size_t ndisplay)
-{
-	struct pollfd pfd;
-	char fdarg[32], line[32];
-	int p[2], n, status;
-	pid_t pid;
-
-	if(pipe(p) < 0)
-		return fail("Xvfb display pipe: %s", strerror(errno));
-	pid = fork();
-	if(pid < 0){
-		close(p[0]);
-		close(p[1]);
-		return fail("fork Xvfb: %s", strerror(errno));
-	}
-	if(pid == 0){
-		close(p[0]);
-		snprintf(fdarg, sizeof fdarg, "%d", p[1]);
-		execlp("Xvfb", "Xvfb", "-displayfd", fdarg, "-screen", "0",
-			"1024x768x24", "-nolisten", "tcp", "-noreset", (char*)NULL);
-		_exit(127);
-	}
-	close(p[1]);
-	pfd.fd = p[0];
-	pfd.events = POLLIN;
-	if(poll(&pfd, 1, Timeout) <= 0){
-		kill(pid, SIGTERM);
-		waitpid(pid, NULL, 0);
-		close(p[0]);
-		return fail("Xvfb did not publish a display");
-	}
-	n = read(p[0], line, sizeof line - 1);
-	close(p[0]);
-	if(n <= 0){
-		waitpid(pid, &status, 0);
-		return fail("read Xvfb display number");
-	}
-	line[n] = '\0';
-	line[strcspn(line, "\r\n")] = '\0';
-	if(snprintf(display, ndisplay, ":%s", line) >= (int)ndisplay){
-		kill(pid, SIGTERM);
-		waitpid(pid, NULL, 0);
-		return fail("Xvfb display number is too long");
-	}
-	*pidp = pid;
-	return 1;
-}
-
-static void
-stopxvfb(pid_t *pidp)
-{
-	int status;
-	int64_t deadline;
-
-	if(*pidp <= 0)
-		return;
-	kill(*pidp, SIGTERM);
-	deadline = nowms() + Timeout;
-	while(waitpid(*pidp, &status, WNOHANG) == 0 && leftms(deadline) > 0)
-		g_usleep(10000);
-	if(waitpid(*pidp, &status, WNOHANG) == 0){
-		kill(*pidp, SIGKILL);
-		waitpid(*pidp, &status, 0);
-	}
-	*pidp = -1;
-}
 
 static void
 pump(void)
@@ -656,16 +589,17 @@ main(int argc, char **argv)
 	static const int badresponses[] = { Rbadcommit, Rbadpreedit };
 	char root[] = "/tmp/strans-gtk.XXXXXX";
 	char runtime[320] = "", socket[384] = "", cache[384] = "";
-	char display[32];
 	char *module;
-	pid_t xvfb;
+	Daemon xvfb;
+	Live l;
 	int first, i, ox, oy, scale, closes, ok, rootmade, serverup;
 	int64_t stalledms, started;
 
 	testname = "gtk_live_test";
 	ctx = oldctx = badctx = NULL;
 	top = NULL;
-	xvfb = -1;
+	memset(&l, 0, sizeof l);
+	daemoninit(&xvfb, "Xvfb");
 	rootmade = 0;
 	serverup = 0;
 	ok = 0;
@@ -687,8 +621,8 @@ main(int argc, char **argv)
 	Check(snprintf(cache, sizeof cache, "%s/immodules.cache", root) <
 		(int)sizeof cache, "cache path is too long");
 	Check(mkdir(runtime, 0700) == 0, "mkdir runtime: %s", strerror(errno));
-	Check(startxvfb(&xvfb, display, sizeof display), "start private Xvfb");
+	Check(startxvfb(&l, &xvfb, "Xvfb"), "start private Xvfb");
-	Check(setenv("DISPLAY", display, 1) == 0 &&
+	Check(setenv("DISPLAY", l.display, 1) == 0 &&
 		setenv("GDK_BACKEND", "x11", 1) == 0 &&
 		setenv("GDK_SCALE", "2", 1) == 0 &&
 		setenv("XDG_RUNTIME_DIR", runtime, 1) == 0,
@@ -951,7 +885,8 @@ out:
 		rmdir(runtime);
 	if(rootmade)
 		rmdir(root);
-	stopxvfb(&xvfb);
+	stopdaemon(&xvfb);
+	closeerrors(&xvfb);
 	free(module);
 	if(ok)
 		printf("gtk_live_test: ok (scale %d, stalled peer %lld ms)\n",

tests/live.c

@@ -240,6 +240,81 @@ childfds(int fd)
 	return dup2(fd, STDOUT_FILENO) >= 0 && dup2(fd, STDERR_FILENO) >= 0;
 }
 
+/*
+ * A private X server for the tests that need one; its display goes in
+ * Live, so the daemon and the clients started after it share it.
+ */
+int
+startxvfb(Live *l, Daemon *d, char *program)
+{
+	struct pollfd pfd;
+	char fdarg[16], line[32];
+	int errpipe[2], n, pipefd[2];
+	int64_t deadline;
+	pid_t pid;
+	ssize_t nr;
+
+	if(pipe2(pipefd, O_CLOEXEC) < 0)
+		return fail("Xvfb display pipe: %s", strerror(errno));
+	if(pipe2(errpipe, O_CLOEXEC|O_NONBLOCK) < 0){
+		close(pipefd[0]);
+		close(pipefd[1]);
+		return fail("pipe2: %s", strerror(errno));
+	}
+	pid = fork();
+	if(pid < 0){
+		close(pipefd[0]);
+		close(pipefd[1]);
+		close(errpipe[0]);
+		close(errpipe[1]);
+		return fail("fork Xvfb: %s", strerror(errno));
+	}
+	if(pid == 0){
+		close(pipefd[0]);
+		close(errpipe[0]);
+		n = fcntl(pipefd[1], F_GETFD);
+		if(n < 0 || fcntl(pipefd[1], F_SETFD, n & ~FD_CLOEXEC) < 0 ||
+		   !childfds(errpipe[1]))
+			_exit(126);
+		snprintf(fdarg, sizeof fdarg, "%d", pipefd[1]);
+		execlp(program, program, "-displayfd", fdarg, "-screen", "0",
+			"1024x768x24", "-nolisten", "tcp", "-noreset", (char*)0);
+		_exit(127);
+	}
+	close(pipefd[1]);
+	close(errpipe[1]);
+	d->pid = pid;
+	d->errfd = errpipe[0];
+	pfd.fd = pipefd[0];
+	pfd.events = POLLIN|POLLHUP;
+	deadline = nowms() + Starttimeout;
+	n = 0;
+	while(n + 1 < (int)sizeof line){
+		pfd.revents = 0;
+		if(poll(&pfd, 1, leftms(deadline)) <= 0)
+			break;
+		nr = read(pipefd[0], line + n, sizeof line - n - 1);
+		if(nr < 0 && errno == EINTR)
+			continue;
+		if(nr <= 0)
+			break;
+		n += nr;
+		line[n] = '\0';
+		if(strchr(line, '\n') != NULL)
+			break;
+	}
+	close(pipefd[0]);
+	if(n == 0 || strchr(line, '\n') == NULL)
+		return fail("Xvfb did not report a private display");
+	line[strcspn(line, "\r\n")] = '\0';
+	if(strspn(line, "0123456789") != strlen(line))
+		return fail("invalid Xvfb display number %s", line);
+	if(snprintf(l->display, sizeof l->display, ":%s", line)
+	   >= (int)sizeof l->display)
+		return fail("Xvfb display number is too long");
+	return 1;
+}
+
 int
 startdaemon(Live *l, Daemon *d, char *program, char *arg)
 {
@@ -268,7 +343,10 @@ startdaemon(Live *l, Daemon *d, char *program, char *arg)
 		}
 		if(setenv("XDG_RUNTIME_DIR", l->runtime, 1) < 0 ||
 		   setenv("XDG_CONFIG_HOME", l->config, 1) < 0 ||
-		   setenv("HOME", l->home, 1) < 0 || unsetenv("DISPLAY") < 0 ||
+		   setenv("HOME", l->home, 1) < 0 ||
+		   (l->display[0] != '\0' ?
+			setenv("DISPLAY", l->display, 1) :
+			unsetenv("DISPLAY")) < 0 ||
 		   unsetenv("DBUS_SESSION_BUS_ADDRESS") < 0 ||
 		   unsetenv("IBUS_ADDRESS") < 0 ||
 		   unsetenv("IBUS_ADDRESS_FILE") < 0){
@@ -393,7 +471,7 @@ stopdaemon(Daemon *d)
 	}
 	/* plan9port turns a caught termination note into exit status 1. */
 	if(reaped && !((WIFSIGNALED(status) && WTERMSIG(status) == SIGTERM) ||
-	   (WIFEXITED(status) && WEXITSTATUS(status) == 1)))
+	   (WIFEXITED(status) && WEXITSTATUS(status) <= 1)))
 		ok = fail("%s exited with unexpected wait status %#x", d->name,
 			status);
 	readerrors(d);

tests/live.h

@@ -39,6 +39,7 @@ struct Live
 	char	home[320];
 	char	socket[384];
 	char	addrfile[512];
+	char	display[32];
 };
 
 extern char *testname;
@@ -54,6 +55,7 @@ int	livesetup(Live*, char*);
 int	liveclean(Live*);
 
 void	daemoninit(Daemon*, char*);
+int	startxvfb(Live*, Daemon*, char*);
 int	startdaemon(Live*, Daemon*, char*, char*);
 int	waitready(Live*, Daemon*, char*, size_t);
 int	daemonalive(Daemon*);

tests/xim_live_test.c

@@ -24,23 +24,14 @@ enum
 	Eventtimeout = 4000,
 };
 
-typedef struct Child Child;
 typedef struct Run Run;
 typedef struct Prelog Prelog;
 
-struct Child
-{
-	pid_t pid;
-	pid_t pgid;
-	char log[320];
-};
-
 struct Run
 {
-	Live l;
+	Live	l;
-	Child xvfb;
+	Daemon	xvfb;
-	Child daemon;
+	Daemon	daemon;
-	char display[32];
 };
 
 struct Prelog
@@ -55,144 +46,10 @@ struct Prelog
 
 static int xerror;
 
-static int
-childlog(char *path)
-{
-	int fd;
-
-	fd = open(path, O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0600);
-	return fd;
-}
-
-static int
-spawnxvfb(Run *r, char *program)
-{
-	char fdstr[16], line[32];
-	int displayfd[2], errfd, flags, n, status;
-	int64_t deadline;
-	pid_t pid;
-	struct pollfd pfd;
-	ssize_t nr;
-
-	if(pipe2(displayfd, O_CLOEXEC) < 0)
-		return fail("create Xvfb display pipe: %s", strerror(errno));
-	errfd = childlog(r->xvfb.log);
-	if(errfd < 0){
-		close(displayfd[0]);
-		close(displayfd[1]);
-		return fail("open Xvfb log: %s", strerror(errno));
-	}
-	pid = fork();
-	if(pid < 0){
-		close(displayfd[0]);
-		close(displayfd[1]);
-		close(errfd);
-		return fail("fork Xvfb: %s", strerror(errno));
-	}
-	if(pid == 0){
-		setpgid(0, 0);
-		close(displayfd[0]);
-		flags = fcntl(displayfd[1], F_GETFD);
-		if(flags < 0 || fcntl(displayfd[1], F_SETFD,
-		   flags & ~FD_CLOEXEC) < 0)
-			_exit(126);
-		if(dup2(errfd, STDOUT_FILENO) < 0 ||
-		   dup2(errfd, STDERR_FILENO) < 0)
-			_exit(126);
-		close(errfd);
-		snprintf(fdstr, sizeof fdstr, "%d", displayfd[1]);
-		execlp(program, program, "-displayfd", fdstr,
-			"-screen", "0", "800x600x24", "-nolisten", "tcp",
-			"-noreset", (char*)0);
-		_exit(127);
-	}
-	close(displayfd[1]);
-	close(errfd);
-	if((setpgid(pid, pid) < 0 && errno != EACCES) || getpgid(pid) != pid){
-		kill(pid, SIGKILL);
-		while(waitpid(pid, &status, 0) < 0 && errno == EINTR)
-			;
-		close(displayfd[0]);
-		return fail("isolate Xvfb process group: %s", strerror(errno));
-	}
-	r->xvfb.pid = pid;
-	r->xvfb.pgid = pid;
-	pfd.fd = displayfd[0];
-	pfd.events = POLLIN|POLLHUP;
-	deadline = nowms() + Starttimeout;
-	n = 0;
-	while(n + 1 < (int)sizeof line){
-		pfd.revents = 0;
-		status = poll(&pfd, 1, leftms(deadline));
-		if(status < 0 && errno == EINTR)
-			continue;
-		if(status <= 0)
-			break;
-		nr = read(displayfd[0], line + n, sizeof line - n - 1);
-		if(nr > 0){
-			n += nr;
-			line[n] = '\0';
-			if(strchr(line, '\n') != NULL)
-				break;
-			continue;
-		}
-		if(nr < 0 && errno == EINTR)
-			continue;
-		break;
-	}
-	close(displayfd[0]);
-	if(n == 0 || strchr(line, '\n') == NULL)
-		return fail("Xvfb did not report a private display");
-	line[strcspn(line, "\r\n")] = '\0';
-	for(n = 0; line[n] != '\0'; n++)
-		if(line[n] < '0' || line[n] > '9')
-			return fail("invalid Xvfb display number %s", line);
-	if(snprintf(r->display, sizeof r->display, ":%s", line)
-	   >= (int)sizeof r->display)
-		return fail("Xvfb display number is too long");
-	return 1;
-}
-
-static int
-spawndaemon(Run *r, char *program, char *mapdir)
-{
-	int errfd;
-	pid_t pid;
-
-	errfd = childlog(r->daemon.log);
-	if(errfd < 0)
-		return fail("open daemon log: %s", strerror(errno));
-	pid = fork();
-	if(pid < 0){
-		close(errfd);
-		return fail("fork daemon: %s", strerror(errno));
-	}
-	if(pid == 0){
-		setpgid(0, 0);
-		if(dup2(errfd, STDOUT_FILENO) < 0 ||
-		   dup2(errfd, STDERR_FILENO) < 0)
-			_exit(126);
-		close(errfd);
-		execl(program, program, mapdir, (char*)0);
-		_exit(127);
-	}
-	close(errfd);
-	if((setpgid(pid, pid) < 0 && errno != EACCES) || getpgid(pid) != pid){
-		kill(pid, SIGKILL);
-		while(waitpid(pid, NULL, 0) < 0 && errno == EINTR)
-			;
-		return fail("isolate daemon process group: %s", strerror(errno));
-	}
-	r->daemon.pid = pid;
-	r->daemon.pgid = pid;
-	return 1;
-}
-
 static int
 waitsocket(Run *r)
 {
 	struct stat st;
-	int status;
 	int64_t deadline;
 
 	deadline = nowms() + Starttimeout;
@@ -200,10 +57,8 @@ waitsocket(Run *r)
 		if(lstat(r->l.socket, &st) == 0 && S_ISSOCK(st.st_mode) &&
 		   (st.st_mode & 0777) == 0600)
 			return 1;
-		if(waitpid(r->daemon.pid, &status, WNOHANG) == r->daemon.pid){
+		if(!daemonalive(&r->daemon))
-			r->daemon.pid = -1;
 			return fail("daemon exited before creating its IPC socket");
-		}
 		pausems(20);
 	}
 	return fail("timed out waiting for daemon IPC socket");
@@ -213,90 +68,29 @@ static int
 start(Run *r, char *daemon, char *mapdir, char *xvfb)
 {
 	memset(r, 0, sizeof *r);
-	r->xvfb.pid = r->daemon.pid = -1;
+	daemoninit(&r->xvfb, "Xvfb");
-	r->xvfb.pgid = r->daemon.pgid = -1;
+	daemoninit(&r->daemon, "daemon");
-	if(!livesetup(&r->l, "xim"))
+	if(!livesetup(&r->l, "xim") || !startxvfb(&r->l, &r->xvfb, xvfb))
 		return 0;
-	if(snprintf(r->xvfb.log, sizeof r->xvfb.log, "%s/xvfb.log", r->l.root)
+	if(setenv("DISPLAY", r->l.display, 1) < 0 ||
-	   >= (int)sizeof r->xvfb.log ||
+	   setenv("XMODIFIERS", "@im=strans", 1) < 0)
-	   snprintf(r->daemon.log, sizeof r->daemon.log, "%s/daemon.log", r->l.root)
-	   >= (int)sizeof r->daemon.log)
-		return fail("temporary path is too long");
-	if(!spawnxvfb(r, xvfb))
-		return 0;
-	if(setenv("DISPLAY", r->display, 1) < 0 ||
-	   setenv("XMODIFIERS", "@im=strans", 1) < 0 ||
-	   setenv("XDG_RUNTIME_DIR", r->l.runtime, 1) < 0 ||
-	   setenv("XDG_CONFIG_HOME", r->l.config, 1) < 0 ||
-	   setenv("HOME", r->l.home, 1) < 0 ||
-	   unsetenv("DBUS_SESSION_BUS_ADDRESS") < 0 ||
-	   unsetenv("IBUS_ADDRESS") < 0)
 		return fail("set private environment: %s", strerror(errno));
-	if(!spawndaemon(r, daemon, mapdir))
+	if(!startdaemon(&r->l, &r->daemon, daemon, mapdir))
 		return 0;
 	return waitsocket(r);
 }
 
-static void
-showlog(char *name)
-{
-	char buf[4096];
-	ssize_t n;
-	int fd;
-
-	fd = open(name, O_RDONLY|O_CLOEXEC);
-	if(fd < 0)
-		return;
-	while((n = read(fd, buf, sizeof buf)) > 0)
-		fwrite(buf, 1, n, stderr);
-	close(fd);
-}
-
-static void
-stopchild(Child *c)
-{
-	int i, status;
-	pid_t n;
-
-	if(c->pgid > 0 && kill(-c->pgid, SIGTERM) < 0 && errno != ESRCH)
-		fail("terminate process group %ld: %s", (long)c->pgid,
-			strerror(errno));
-	if(c->pid > 0)
-		for(i = 0; i < Stoptimeout / 20; i++){
-			n = waitpid(c->pid, &status, WNOHANG);
-			if(n == c->pid || (n < 0 && errno == ECHILD)){
-				c->pid = -1;
-				break;
-			}
-			if(n < 0 && errno != EINTR)
-				break;
-			pausems(20);
-		}
-	if(c->pgid > 0)
-		kill(-c->pgid, SIGKILL);
-	if(c->pid > 0){
-		while(waitpid(c->pid, &status, 0) < 0 && errno == EINTR)
-			;
-		c->pid = -1;
-	}
-	c->pgid = -1;
-}
-
 static void
 cleanup(Run *r, int noisy)
 {
-	stopchild(&r->daemon);
+	stopdaemon(&r->daemon);
-	stopchild(&r->xvfb);
+	stopdaemon(&r->xvfb);
 	if(noisy){
-		fprintf(stderr, "%s: daemon log:\n", testname);
+		showerrors(&r->daemon);
-		showlog(r->daemon.log);
+		showerrors(&r->xvfb);
-		fprintf(stderr, "%s: Xvfb log:\n", testname);
-		showlog(r->xvfb.log);
 	}
-	if(r->daemon.log[0] != '\0')
+	closeerrors(&r->daemon);
-		unlink(r->daemon.log);
+	closeerrors(&r->xvfb);
-	if(r->xvfb.log[0] != '\0')
-		unlink(r->xvfb.log);
 	liveclean(&r->l);
 }
 
@@ -765,9 +559,6 @@ main(int argc, char **argv)
 		fprintf(stderr, "usage: xim_live_test daemon mapdir [Xvfb]\n");
 		return 2;
 	}
-	memset(&run, 0, sizeof run);
-	run.xvfb.pid = run.daemon.pid = -1;
-	run.xvfb.pgid = run.daemon.pgid = -1;
 	xvfb = argc == 4 ? argv[3] : "Xvfb";
 	ok = start(&run, argv[1], argv[2], xvfb);
 	if(!ok){
@@ -781,15 +572,15 @@ main(int argc, char **argv)
 		return 1;
 	}
 	XSetErrorHandler(xerr);
-	dpy = XOpenDisplay(run.display);
+	dpy = XOpenDisplay(run.l.display);
 	if(dpy == NULL){
-		fail("open private display %s", run.display);
+		fail("open private display %s", run.l.display);
 		cleanup(&run, 1);
 		return 1;
 	}
 	im = openim(dpy);
 	if(im == NULL){
-		fail("open strans XIM on private display %s", run.display);
+		fail("open strans XIM on private display %s", run.l.display);
 		XCloseDisplay(dpy);
 		cleanup(&run, 1);
 		return 1;