Skip to content

Commit 4d9eeb9

Browse files
committed
ws-over-h2: don't conflate nwsi buffered output with the stream's parked frames
The tx flow-control parking introduced in 57effc6 gates the park, drain and re-arm logic on lws_has_buffered_out(), which for an h2 stream also reports the NETWORK wsi's buflist_out -- the buffer that fills whenever lws_issue_raw() takes a partial socket write. The drain logic, however, assumes the predicate describes the stream's OWN parked frames, which breaks four ways under ordinary socket-level backpressure: - lws_h2_ws_drain_parked_tx(): after the stream's last parked frame is consumed, a partial write left by its own lws_issue_raw(nwsi) keeps the while (lws_has_buffered_out(wsi)) loop alive via the nwsi branch; the next iteration reads the empty stream buflist and returns -1, and rops_perform_user_POLLOUT_h2() closes a healthy stream that had just flushed everything it owned. - rops_perform_user_POLLOUT_h2(): the post-drain re-arm is skipped whenever the nwsi still holds a partial, on the assumption that a WINDOW_UPDATE will re-arm the child. When the peer's window is large (tx credit ample, bottleneck is the socket) no WINDOW_UPDATE is coming, requested_POLLOUT was already consumed, and the pending- writable accounting then drops POLLOUT on the connection: the user callback never fires again and the transfer stalls permanently. - the same gate pulls a SIBLING ws stream -- nothing of its own parked, POLLOUT requested -- into the drain once an earlier child's drain congests the nwsi in the same service pass, hitting the identical empty-buflist close. - lws_h2_frame_write(): the parking predicate fires on nwsi congestion caused by another stream, needlessly deferring fully-credited frames into the drain path instead of letting lws_issue_raw(nwsi) append them in order. Test the stream's own buflist_out directly in all four places, and let a carries-ws child with nothing of its own parked fall through to normal servicing. The existing api-test doesn't catch these because its drip-fed WINDOW_UPDATEs continually re-arm every child and localhost doesn't produce partial writes.
1 parent f3a9f48 commit 4d9eeb9

2 files changed

Lines changed: 24 additions & 9 deletions

File tree

lib/roles/h2/http2.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -706,7 +706,7 @@ int lws_h2_frame_write(struct lws *wsi, int type, int flags,
706706
if (type == LWS_H2_FRAME_TYPE_DATA) {
707707
#if defined(LWS_ROLE_WS)
708708
if (wsi->h23_stream_carries_ws &&
709-
(lws_has_buffered_out(wsi) ||
709+
(wsi->buflist_out ||
710710
lws_h2_tx_cr_get(wsi) < (int)len)) {
711711
/*
712712
* ws-over-h2 (RFC 8441): not enough h2 flow-control
@@ -768,7 +768,13 @@ lws_h2_ws_drain_parked_tx(struct lws *nwsi, struct lws *wsi)
768768
{
769769
uint8_t out[LWS_H2_FRAME_HEADER_LENGTH + 4096];
770770

771-
while (lws_has_buffered_out(wsi)) {
771+
/*
772+
* Test the stream's own buflist directly: lws_has_buffered_out()
773+
* additionally reports the nwsi's buffer, which fills up whenever
774+
* lws_issue_raw() below takes a partial socket write and must not
775+
* keep us looping after our own parked frames are gone.
776+
*/
777+
while (wsi->buflist_out) {
772778
uint8_t *seg = NULL;
773779
size_t sl = lws_buflist_next_segment_len(&wsi->buflist_out,
774780
&seg);

lib/roles/h2/ops-h2.c

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1089,13 +1089,20 @@ rops_perform_user_POLLOUT_h2(struct lws *wsi)
10891089

10901090
/* priority 1: post compression-transform buffered output */
10911091

1092-
if (lws_has_buffered_out(w)) {
10931092
#if defined(LWS_ROLE_WS)
1094-
if (w->h23_stream_carries_ws) {
1095-
/*
1096-
* ws-over-h2: whole DATA frames parked by
1097-
* lws_h2_frame_write() waiting for tx credit
1098-
*/
1093+
if (w->h23_stream_carries_ws) {
1094+
/*
1095+
* ws-over-h2: whole DATA frames parked by
1096+
* lws_h2_frame_write() waiting for tx credit. Gate
1097+
* and re-arm on the stream's OWN buflist, not
1098+
* lws_has_buffered_out(): that also reports the
1099+
* nwsi's buffer (eg, another stream's partial socket
1100+
* write), which must neither pull us into the drain
1101+
* with nothing parked nor block re-arming the user
1102+
* once our own frames are gone. If nothing of ours
1103+
* is parked, fall through to normal servicing.
1104+
*/
1105+
if (w->buflist_out) {
10991106
if (lws_h2_ws_drain_parked_tx(wsi, w) < 0) {
11001107
lwsl_info("%s signalling to close\n",
11011108
__func__);
@@ -1105,7 +1112,7 @@ rops_perform_user_POLLOUT_h2(struct lws *wsi)
11051112
wa = &wsi->mux.child_list;
11061113
goto next_child;
11071114
}
1108-
if (!lws_has_buffered_out(w))
1115+
if (!w->buflist_out)
11091116
/* fully drained: let the user write */
11101117
lws_callback_on_writable(w);
11111118
/*
@@ -1115,7 +1122,9 @@ rops_perform_user_POLLOUT_h2(struct lws *wsi)
11151122
wa = &wsi->mux.child_list;
11161123
goto next_child;
11171124
}
1125+
} else
11181126
#endif
1127+
if (lws_has_buffered_out(w)) {
11191128
lwsl_debug("%s: completing partial\n", __func__);
11201129
if (lws_issue_raw(w, NULL, 0) < 0) {
11211130
lwsl_info("%s signalling to close\n", __func__);

0 commit comments

Comments
 (0)