[OpenWrt-Devel] [PATCH procd] service: use PTY (instead of a pipe) for reading service output

[Rafał Miłecki]

Using pipe automatically switches service to block buffering which kind
of breaks our logging. We won't get anything from FD until the buffer
gets filled fully or the service exits. This makes log messages appear
with an unwanted delay.
Switching to PTY fixes this issue. Service prints to the "standard"
stdout (no piping) but the messages go (line buffered) to the master FD
anyway.
It was successfully tested with all 4 variants of "stdout" and "stderr"
ubus arguments.

Signed-off-by: Rafał Miłecki <zajec5 at gmail.com>
---
 CMakeLists.txt     |  2 +-
 service/instance.c | 35 +++++++++++++++++++++++++----------
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index dfa9413..7e1fad3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -14,7 +14,7 @@ SET(SOURCES procd.c signal.c watchdog.c state.c	inittab.c rcS.c	ubus.c system.c
 	service/service.c service/instance.c service/validate.c service/trigger.c service/watch.c
 	plug/coldplug.c plug/hotplug.c utils/utils.c)
 
-SET(LIBS ubox ubus json-c blobmsg_json json_script)
+SET(LIBS util ubox ubus json-c blobmsg_json json_script)
 
 IF(DEBUG)
   ADD_DEFINITIONS(-DDEBUG -g3)
diff --git a/service/instance.c b/service/instance.c
index 35b2def..110db36 100644
--- a/service/instance.c
+++ b/service/instance.c
@@ -23,6 +23,8 @@
 #include <pwd.h>
 #include <libgen.h>
 #include <unistd.h>
+#include <pty.h>
+#include <utmp.h>
 
 #include <libubox/md5.h>
 
@@ -273,7 +275,7 @@ instance_run(struct service_instance *in, int _stdout, int _stderr)
 		dup2(_stdin, STDIN_FILENO);
 		closefd(_stdin);
 	}
-	if (_stdout > -1) {
+	if (_stdout > -1 && _stdout != STDOUT_FILENO) {
 		dup2(_stdout, STDOUT_FILENO);
 		closefd(_stdout);
 	}
@@ -314,8 +316,9 @@ instance_free_stdio(struct service_instance *in)
 void
 instance_start(struct service_instance *in)
 {
+	int amaster = -1, aslave = -1;
 	int pid;
-	int opipe[2] = { -1, -1 };
+	int _stdout = -1;
 	int epipe[2] = { -1, -1 };
 
 	if (!avl_is_empty(&in->errors.avl)) {
@@ -327,13 +330,19 @@ instance_start(struct service_instance *in)
 		return;
 
 	instance_free_stdio(in);
+
+	/* We don't want block buffered stdout so use PTY instead of a pipe */
 	if (in->_stdout.fd.fd > -2) {
-		if (pipe(opipe)) {
-			ULOG_WARN("pipe() failed: %d (%s)\n", errno, strerror(errno));
-			opipe[0] = opipe[1] = -1;
+		if (openpty(&amaster, &aslave, NULL, NULL, NULL)) {
+			ULOG_WARN("openpty() failed: %d (%s)\n", errno, strerror(errno));
+			amaster = aslave = -1;
+		} else {
+			/* Child should just use stdout which will go to the master */
+			_stdout = STDOUT_FILENO;
 		}
 	}
 
+	/* Use pipe as stderr is always unbuffered by default */
 	if (in->_stderr.fd.fd > -2) {
 		if (pipe(epipe)) {
 			ULOG_WARN("pipe() failed: %d (%s)\n", errno, strerror(errno));
@@ -353,9 +362,15 @@ instance_start(struct service_instance *in)
 
 	if (!pid) {
 		uloop_done();
-		closefd(opipe[0]);
+		close(amaster);
 		closefd(epipe[0]);
-		instance_run(in, opipe[1], epipe[1]);
+		if (aslave > -1) {
+			if (login_tty(aslave)) {
+				ULOG_WARN("login_tty() failed: %d (%s)\n", errno, strerror(errno));
+				_stdout = -1;
+			}
+		}
+		instance_run(in, _stdout, epipe[1]);
 		return;
 	}
 
@@ -364,9 +379,9 @@ instance_start(struct service_instance *in)
 	clock_gettime(CLOCK_MONOTONIC, &in->start);
 	uloop_process_add(&in->proc);
 
-	if (opipe[0] > -1) {
-		ustream_fd_init(&in->_stdout, opipe[0]);
-		closefd(opipe[1]);
+	if (amaster > -1) {
+		ustream_fd_init(&in->_stdout, amaster);
+		closefd(aslave);
 	}
 
 	if (epipe[0] > -1) {
-- 
1.8.4.5
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel