BUG/MEDIUM: h2: work around a connection API limitation
The connection API permits us to enable or disable receiving on a
connection. The underlying FD layer arranges this with the polling
and the fd cache. In practice, if receiving was allowed and an end
of buffer was reached, the FD is subscribed to the polling. If later
we want to process pending data from the buffer, we have to enable
receiving again, but since it's already enabled (in polled mode),
nothing happens and the pending data remain stuck until a new event
happens on the connection to wake the FD up. This is a limitation of
the internal connection API which is not very friendly to the new mux
architecture.
The visible effect is that certain uploads to slow servers experience
truncation on timeout on their last blocks because nothing new comes
from the connection to wake it up while it's being polled.
In order to work around this, there are two solutions :
- either cheat on the connection so that conn_update_xprt_polling()
always performs a call to fd_may_recv() after fd_want_recv(), that
we can trigger from the mux by always calling conn_xprt_stop_recv()
before conn_xprt_want_recv(), but that's a bit tricky and may have
side effects on other parts (eg: SSL)
- or we refrain from receiving in the mux as soon as we're busy on
anything else, regardless of whether or not some room is available
in the receive buffer.
This patch takes the second approach above. This way once we read some
data, as soon as we detect that we're stuck, we immediately stop receiving.
This ensures the event doesn't go into polled mode for this period and
that as soon as we're unstuck we can continue. In fact this guarantees
that we can only wait on one side of the mux for a given direction. A
future improvement of the connection layer should make it possible to
resume processing of an interrupted receive operation.
This fix must be backported to 1.8.
diff --git a/src/mux_h2.c b/src/mux_h2.c
index fc655db..570773b 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -226,8 +226,13 @@
* is empty, we must not attempt to receive
* - if the demux buf failed to be allocated, we must not try to receive and
* we know there is nothing pending
- * - if the buffer is not full, we may attempt to receive
- * - if no flag indicates a blocking condition, we may attempt to receive
+ * - if no flag indicates a blocking condition, we may attempt to receive,
+ * regardless of whether the demux buffer is full or not, so that only
+ * de demux part decides whether or not to block. This is needed because
+ * the connection API indeed prevents us from re-enabling receipt that is
+ * already enabled in a polled state, so we must always immediately stop
+ * as soon as the demux can't proceed so as never to hit an end of read
+ * with data pending in the buffers.
* - otherwise must may not attempt
*/
static inline int h2_recv_allowed(const struct h2c *h2c)
@@ -239,8 +244,7 @@
return 0;
if (!(h2c->flags & H2_CF_DEM_DALLOC) &&
- (!(h2c->flags & H2_CF_DEM_DFULL) ||
- !(h2c->flags & H2_CF_DEM_BLOCK_ANY)))
+ !(h2c->flags & H2_CF_DEM_BLOCK_ANY))
return 1;
return 0;