dovecot - Jan 2023 - doveadm sync - I/O has stalled

Hi,
> But when dsync connects over tcps,
> it uses function implementations in lib-ssl-iostream to send and receive
data.
> And this simplified call stack would result in some data are read when
calling o_stream_uncork
> 
> o_stream_uncork => o_stream_flush => o_stream_ssl_flush_buffer =>
openssl_iostream_bio_sync => openssl_iostream_bio_input
> 
> If some data arrive after ibc->ibc.io_callback(ibc->ibc.io_context)
and before o_stream_uncork,
> o_stream_uncork would read the data and then return.
> After o_stream_uncork returns, dsync then waits for new data to be read or
written.
> But because the data had been read in o_stream_uncork, and there may be no
new data to be read,
> dsync may then wait until timeout is met.
If this happens, it's supposed to mark the IO as having data
"pending", which should prevent the timeout. The logic seems to me
like it should work, although I'm now wondering if it can hang when there is
some internal SSL state data that needs to be read even though there is no
actual data being sent. So:
> It may happen, but it is hard to reproduce.
Are you able to try if this patch happens to fix it? :

diff --git a/src/lib-ssl-iostream/iostream-openssl.c
b/src/lib-ssl-iostream/iostream-openssl.c
index fa9afb22f7..094abaa56c 100644
--- a/src/lib-ssl-iostream/iostream-openssl.c
+++ b/src/lib-ssl-iostream/iostream-openssl.c
@@ -550,10 +550,7 @@ openssl_iostream_bio_input(struct ssl_iostream *ssl_io,
                }
        }
        if (bytes_read || i_stream_get_data_size(ssl_io->plain_input) > 0)
{
-               if (i_stream_get_data_size(ssl_io->plain_input) > 0 ||
-                   (type != OPENSSL_IOSTREAM_SYNC_TYPE_FIRST_READ &&
-                    type != OPENSSL_IOSTREAM_SYNC_TYPE_CONTINUE_READ))
-                       i_stream_set_input_pending(ssl_io->ssl_input, TRUE);
+               i_stream_set_input_pending(ssl_io->ssl_input, TRUE);
                ssl_io->want_read = FALSE;
        }
        return (bytes_read ? 1 : 0);

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<https://dovecot.org/pipermail/dovecot/attachments/20230102/286f9eb4/attachment.htm>

On 2. Jan 2023, at 3.21, Timo Sirainen <timo at sirainen.com>
wrote:
> 
> 
>> It may happen, but it is hard to reproduce.
> 
> Are you able to try if this patch happens to fix it? :
Actually, looking at it slightly more, there may be another bug in it, although
I'm not sure how it affects the behavior. So try this instead - I'll
also run it through our CI to see if it causes any issues:

diff --git a/src/lib-ssl-iostream/iostream-openssl.c
b/src/lib-ssl-iostream/iostream-openssl.c
index fa9afb22f7..0b22a07a84 100644
--- a/src/lib-ssl-iostream/iostream-openssl.c
+++ b/src/lib-ssl-iostream/iostream-openssl.c
@@ -548,14 +548,10 @@ openssl_iostream_bio_input(struct ssl_iostream *ssl_io,
                        ssl_io->ostream_flush_waiting_input = FALSE;
                        o_stream_set_flush_pending(ssl_io->plain_output,
TRUE);
                }
-       }
-       if (bytes_read || i_stream_get_data_size(ssl_io->plain_input) > 0)
{
-               if (i_stream_get_data_size(ssl_io->plain_input) > 0 ||
-                   (type != OPENSSL_IOSTREAM_SYNC_TYPE_FIRST_READ &&
-                    type != OPENSSL_IOSTREAM_SYNC_TYPE_CONTINUE_READ))
-                       i_stream_set_input_pending(ssl_io->ssl_input, TRUE);
                ssl_io->want_read = FALSE;
        }
+       if (bytes_read || i_stream_get_data_size(ssl_io->plain_input) > 0)
+               i_stream_set_input_pending(ssl_io->ssl_input, TRUE);
        return (bytes_read ? 1 : 0);
 }