BUG/MINOR: mux-quic: properly handle STREAM frame alloc failure
Previously, if a STREAM frame cannot be allocated for emission, a crash
would occurs due to an ABORT_NOW() statement in _qc_send_qcs().
Replace this by proper error code handling. Each stream were sending
fails are removed temporarily from qcc::send_list to a list local to
_qc_send_qcs(). Once emission has been conducted for all streams,
reinsert failed stream to qcc::send_list. This avoids to reloop on
failed streams on the second while loop at the end of _qc_send_qcs().
This crash was reproduced using -dMfail.
This should be backported up to 2.6.
diff --git a/src/mux_quic.c b/src/mux_quic.c
index a74aa95..00b83a1 100644
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -1785,7 +1785,8 @@
* is then generated and inserted in <frms> list.
*
* Returns the total bytes transferred between qcs and quic_stream buffers. Can
- * be null if out buffer cannot be allocated.
+ * be null if out buffer cannot be allocated. On error a negative error code is
+ * used.
*/
static int _qc_send_qcs(struct qcs *qcs, struct list *frms)
{
@@ -1835,14 +1836,17 @@
/* Build a new STREAM frame with <out> buffer. */
if (qcs->tx.sent_offset != qcs->tx.offset || fin) {
- int ret;
- ret = qcs_build_stream_frm(qcs, out, fin, frms);
- if (ret < 0) { ABORT_NOW(); /* TODO handle this properly */ }
+ if (qcs_build_stream_frm(qcs, out, fin, frms) < 0)
+ goto err;
}
out:
TRACE_LEAVE(QMUX_EV_QCS_SEND, qcc->conn, qcs);
return xfer;
+
+ err:
+ TRACE_DEVEL("leaving on error", QMUX_EV_QCS_SEND, qcc->conn, qcs);
+ return -1;
}
/* Proceed to sending. Loop through all available streams for the <qcc>
@@ -1853,8 +1857,10 @@
static int qc_send(struct qcc *qcc)
{
struct list frms = LIST_HEAD_INIT(frms);
+ /* Temporary list for QCS on error. */
+ struct list qcs_failed = LIST_HEAD_INIT(qcs_failed);
struct qcs *qcs, *qcs_tmp;
- int total = 0;
+ int ret, total = 0;
TRACE_ENTER(QMUX_EV_QCC_SEND, qcc->conn);
@@ -1913,8 +1919,16 @@
continue;
}
- if (!(qcs->flags & QC_SF_BLK_SFCTL))
- total += _qc_send_qcs(qcs, &frms);
+ if (!(qcs->flags & QC_SF_BLK_SFCTL)) {
+ if ((ret = _qc_send_qcs(qcs, &frms)) < 0) {
+ /* Temporarily remove QCS from send-list. */
+ LIST_DEL_INIT(&qcs->el_send);
+ LIST_APPEND(&qcs_failed, &qcs->el_send);
+ continue;
+ }
+
+ total += ret;
+ }
}
/* Retry sending until no frame to send, data rejected or connection
@@ -1924,15 +1938,22 @@
/* Reloop over <qcc.send_list>. Useful for streams which have
* fulfilled their qc_stream_desc buf and have now release it.
*/
- list_for_each_entry(qcs, &qcc->send_list, el_send) {
+ list_for_each_entry_safe(qcs, qcs_tmp, &qcc->send_list, el_send) {
/* Only streams blocked on flow-control or waiting on a
* new qc_stream_desc should be present in send_list as
* long as transport layer can handle all data.
*/
BUG_ON(qcs->stream->buf && !(qcs->flags & QC_SF_BLK_SFCTL));
- if (!(qcs->flags & QC_SF_BLK_SFCTL))
- total += _qc_send_qcs(qcs, &frms);
+ if (!(qcs->flags & QC_SF_BLK_SFCTL)) {
+ if ((ret = _qc_send_qcs(qcs, &frms)) < 0) {
+ LIST_DEL_INIT(&qcs->el_send);
+ LIST_APPEND(&qcs_failed, &qcs->el_send);
+ continue;
+ }
+
+ total += ret;
+ }
}
}
@@ -1945,6 +1966,17 @@
qc_frm_free(&frm);
}
+ /* Re-insert on-error QCS at the end of the send-list. */
+ if (!LIST_ISEMPTY(&qcs_failed)) {
+ list_for_each_entry_safe(qcs, qcs_tmp, &qcs_failed, el_send) {
+ LIST_DEL_INIT(&qcs->el_send);
+ LIST_APPEND(&qcc->send_list, &qcs->el_send);
+ }
+
+ if (!(qcc->flags & QC_CF_BLK_MFCTL))
+ tasklet_wakeup(qcc->wait_event.tasklet);
+ }
+
TRACE_LEAVE(QMUX_EV_QCC_SEND, qcc->conn);
return total;