lib: Fix potential hangs when filter istream read doesn't read from its parent

More specifically, this fixes at least a hang where:
 * SSL istream reads some data into buffer
 * SSL iostream places input into internal BIO buffer, which doesn't yet
   get copied to the SSL istream's buffer
 * zlib_istream gets more data from parent SSL istream. Since there is already
   some data buffered, it doesn't call the parent read(). At the end it checks
   that parent SSL istream has 0 bytes in its buffer, so it doesn't se the
   IO pending.

The problem is that SSL istream would buffer if its read() had been called.
Since there is no more IO pending, it causes a hang.

Alternative rejected ideas I thought of:
 * Add some new i_stream_has_maybe_more_data() method that all istreams
   need to implement and filter istreams need to call and use it to set IO
   pending.
 * Change SSL iostream to immediately place input into SSL istream.
   However, this makes the following i_stream_read() behavior confusing,
   since it won't return the newly added data, and might even return -2
   as buffer full, which the caller might not handle properly. Or if
   i_stream_read() did return the newly added data, it would be wrong for
   i_stream_get_data*() to return the newly added data before, effectively
   making the whole change pointless.

diff --git a/src/lib/istream-private.h b/src/lib/istream-private.h
index b612b9c7f2a95aa28e9fbb051b88908ea50a0792..7e0e1f0b61fe537eb887820d8c79eeb7b55226d2 100644 (file)
--- a/src/lib/istream-private.h
+++ b/src/lib/istream-private.h
@@ -67,7 +67,18 @@ struct istream_private {
        bool return_nolf_line:1;
        bool stream_size_passthrough:1; /* stream is parent's size */
        bool nonpersistent_buffers:1;
+       /* After IO is added back to this istream, call io_set_pending() for
+          it. This is set only for the root istream. */
        bool io_pending:1;
+       /* If this is TRUE for the istream or its parents after i_stream_read(),
+          set the istream IO pending again. This is cleared before each
+          istream read(). The purpose is to prevent hangs in case a child
+          istream has set the IO pending, but a parent istream read() won't
+          call the child istream's read() e.g. because its internal buffer is
+          already full. In such situation the IO must be set pending again.
+          This is especially necessary when the istream doesn't otherwise make
+          it visible that it has buffered data, such as ssl-istream. */
+       bool io_pending_until_read:1;
 };
 
 struct istream_snapshot {

diff --git a/src/lib/istream.c b/src/lib/istream.c
index e1687d42e04845104e537282b1dd0ec1a5e281b8..b13f3f43ae10d309202c4ba89deb822e3a520323 100644 (file)
--- a/src/lib/istream.c
+++ b/src/lib/istream.c
@@ -281,6 +281,13 @@ i_stream_noop_snapshot(struct istream_private *stream ATTR_UNUSED,
        return prev_snapshot;
 }
 
+static bool i_stream_is_io_pending_until_read(struct istream_private *_stream)
+{
+       while (_stream->parent != NULL && !_stream->io_pending_until_read)
+               _stream = _stream->parent->real_stream;
+       return _stream->io_pending_until_read;
+}
+
 ssize_t i_stream_read(struct istream *stream)
 {
        struct istream_private *_stream = stream->real_stream;
@@ -324,6 +331,13 @@ ssize_t i_stream_read(struct istream *stream)
                }
        }
 #endif
+       if (!_stream->istream.eof &&
+           i_stream_is_io_pending_until_read(_stream)) {
+               /* One of the parent istreams still has IO pending, because its
+                  read() wasn't called. Set IO back to pending to prevent
+                  hangs. */
+               i_stream_set_input_pending(stream, TRUE);
+       }
        return ret;
 }
 
@@ -352,6 +366,7 @@ ssize_t i_stream_read_memarea(struct istream *stream)
                _stream->high_pos = 0;
        } else {
                _stream->high_pos = 0;
+               _stream->io_pending_until_read = FALSE;
                ret = _stream->read(_stream);
        }
        i_assert(old_size <= _stream->pos - _stream->skip);
@@ -1012,6 +1027,8 @@ void i_stream_set_input_pending(struct istream *stream, bool pending)
        if (!pending)
                return;
 
+       stream->real_stream->io_pending_until_read = TRUE;
+
        stream = i_stream_get_root_io(stream);
        if (stream->real_stream->io != NULL)
                io_set_pending(stream->real_stream->io);