From: Timo Sirainen <timo.sirainen@dovecot.fi>
Date: Mon, 18 Jun 2018 13:07:30 +0000 (+0300)
Subject: lib: Revert earlier "log write is blocking" handling change
X-Git-Tag: 2.3.2.rc1~3
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ad5447e35bc9028f595fe067599bc632065e2601;p=thirdparty%2Fdovecot%2Fcore.git

lib: Revert earlier "log write is blocking" handling change

1) It was buggy, because it set O_NONBLOCK rather than removing it.

2) fd flags are shared across all the processes using the fd. We can't
reliably implement the process title update, because some processes are
blocking on the log write() before they change the process title.

Reverts 861d40b9aebabccae7d48e49a18cbc631ab1fefe
---

diff --git a/src/lib/failures.c b/src/lib/failures.c
index f950eb0652..37f36a7c63 100644
--- a/src/lib/failures.c
+++ b/src/lib/failures.c
@@ -105,17 +105,20 @@ static void log_prefix_add(const struct failure_context *ctx, string_t *str)
 		str_append(str, log_prefix);
 }
 
+static void log_fd_flush_stop(struct ioloop *ioloop)
+{
+	io_loop_stop(ioloop);
+}
+
 static int log_fd_write(int fd, const unsigned char *data, size_t len)
 {
+	struct ioloop *ioloop;
+	struct io *io;
 	ssize_t ret;
 	unsigned int prev_signal_term_counter = signal_term_counter;
 	unsigned int terminal_eintr_count = 0;
-	const char *old_title = NULL;
-	int fd_orig_flags = -1;
-	bool failed = FALSE;
 
-	while (!failed &&
-	       (ret = write(fd, data, len)) != (ssize_t)len) {
+	while ((ret = write(fd, data, len)) != (ssize_t)len) {
 		if (ret > 0) {
 			/* some was written, continue.. */
 			data += ret;
@@ -125,19 +128,17 @@ static int log_fd_write(int fd, const unsigned char *data, size_t len)
 		if (ret == 0) {
 			/* out of disk space? */
 			errno = ENOSPC;
-			failed = TRUE;
-			break;
+			return -1;
 		}
 		switch (errno) {
 		case EAGAIN: {
-			/* Update the process title to show that we're waiting
-			   on log writing. Switch to blocking writes and retry.
-			   When logging is finished, restore the process
-			   title. */
-			const char *title;
-
-			if (old_title == NULL)
-				old_title = t_strdup(process_title_get());
+			/* wait until we can write more. this can happen at
+			   least when writing to terminal, even if fd is
+			   blocking. Internal logging fd is also now
+			   non-blocking, so we can show warnings about blocking
+			   on a log write. */
+			const char *title, *old_title =
+				t_strdup(process_title_get());
 
 			if (old_title == NULL)
 				title = "[blocking on log write]";
@@ -146,22 +147,13 @@ static int log_fd_write(int fd, const unsigned char *data, size_t len)
 							old_title);
 			process_title_set(title);
 
-			/* Now that the process title is updated, set fd to be
-			   blocking and retry. */
-			if (fd_orig_flags == -1) {
-				fd_orig_flags = fcntl(fd, F_GETFL, 0);
-				if (fd_orig_flags < 0)
-					i_fatal("fcntl(%d, F_GETFL) failed: %m", fd);
-
-				if ((fd_orig_flags & O_NONBLOCK) == 0) {
-					/* EAGAIN on a blocking fd? This is a
-					   bit weird, but just continue. */
-				} else {
-					int flags = fd_orig_flags | O_NONBLOCK;
-					if (fcntl(fd, F_SETFL, flags) < 0)
-						i_fatal("fcntl(%d, F_SETFL) failed: %m", fd);
-				}
-			}
+			ioloop = io_loop_create();
+			io = io_add(fd, IO_WRITE, log_fd_flush_stop, ioloop);
+			io_loop_run(ioloop);
+			io_remove(&io);
+			io_loop_destroy(&ioloop);
+
+			process_title_set(old_title);
 			break;
 		}
 		case EINTR:
@@ -173,23 +165,15 @@ static int log_fd_write(int fd, const unsigned char *data, size_t len)
 			} else {
 				/* received two terminal signals.
 				   someone wants us dead. */
-				failed = TRUE;
-				break;
+				return -1;
 			}
 			break;
 		default:
-			failed = TRUE;
-			break;
+			return -1;
 		}
 		prev_signal_term_counter = signal_term_counter;
 	}
-	if (fd_orig_flags != -1) {
-		if (fcntl(fd, F_SETFL, fd_orig_flags) < 0)
-			i_fatal("fcntl(%d, F_SETFL) failed: %m", fd);
-	}
-	if (old_title != NULL)
-		process_title_set(old_title);
-	return failed ? -1 : 0;
+	return 0;
 }
 
 static int ATTR_FORMAT(3, 0)