]> git.ipfire.org Git - thirdparty/dovecot/core.git/commitdiff
lib: Revert earlier "log write is blocking" handling change
authorTimo Sirainen <timo.sirainen@dovecot.fi>
Mon, 18 Jun 2018 13:07:30 +0000 (16:07 +0300)
committerTimo Sirainen <timo.sirainen@dovecot.fi>
Mon, 18 Jun 2018 18:12:12 +0000 (21:12 +0300)
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

src/lib/failures.c

index f950eb0652a232f45d2a30abcaf10817ce4f2d69..37f36a7c63492b3052c42c93d46d5654ef29b369 100644 (file)
@@ -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)