On 5/27/20 5:25 PM, Stefan Eissing wrote:
> Maybe this can work? It goes into blocking read with default timeout when
> there is definitely nothing to send from our end.
>
>
> h2-proxy-timeout.patch
>
> Index: modules/http2/h2_proxy_session.c
> ===================================================================
> --- modules/http2/h2_proxy_session.c (Revision 1878161)
> +++ modules/http2/h2_proxy_session.c (Arbeitskopie)
> @@ -556,11 +556,14 @@
> "h2_proxy_stream(%d): request DATA %ld, %ld"
> " total, flags=%d", stream->id, (long)readlen,
> (long)stream->data_sent,
> (int)*data_flags);
> - if ((*data_flags & NGHTTP2_DATA_FLAG_EOF) &&
> !apr_is_empty_table(stream->r->trailers_in)) {
> - ap_log_rerror(APLOG_MARK, APLOG_DEBUG, status, stream->r,
> APLOGNO(10179)
> - "h2_proxy_stream(%d): submit trailers",
> stream->id);
> - *data_flags |= NGHTTP2_DATA_FLAG_NO_END_STREAM;
> - submit_trailers(stream);
> + if (*data_flags & NGHTTP2_DATA_FLAG_EOF) {
> + if (!apr_is_empty_table(stream->r->trailers_in)) {
> + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, status, stream->r,
> APLOGNO(10179)
> + "h2_proxy_stream(%d): submit trailers",
> stream->id);
> + *data_flags |= NGHTTP2_DATA_FLAG_NO_END_STREAM;
> + submit_trailers(stream);
> + }
> + stream->state = H2_STREAM_ST_CLOSED_INPUT;
Where is this state processed? Looks to me as if this state would fall into the
default case of the switch in
h2_proxy_session_process and thus cause an error messages. But as far as I
understand this only indicates that the front does not
want to sent more data which is no error.
> }
> return readlen;
> }
> @@ -902,7 +905,7 @@
> apr_socket_t *socket = NULL;
> apr_time_t save_timeout = -1;
>
> - if (block) {
> + if (block && timeout > 0) {
> socket = ap_get_conn_socket(session->c);
> if (socket) {
> apr_socket_timeout_get(socket, &save_timeout);
> @@ -974,6 +977,11 @@
> dispatch_event(session, H2_PROXYS_EV_STREAM_RESUMED, 0, NULL);
> }
>
> +static int has_suspended_streams(h2_proxy_session *session)
> +{
> + return (session->suspended->nelts > 0);
> +}
> +
> static apr_status_t check_suspended(h2_proxy_session *session)
> {
> h2_proxy_stream *stream;
> @@ -1428,7 +1436,20 @@
> break;
>
> case H2_PROXYS_ST_WAIT:
> - if (check_suspended(session) == APR_EAGAIN) {
> + if (!has_suspended_streams(session)
> + && !nghttp2_session_want_write(session->ngh2)
> + && nghttp2_session_want_read(session->ngh2)) {
> + status = h2_proxy_session_read(session, 1, 0);
> + if (status == APR_SUCCESS) {
> + have_read = 1;
> + dispatch_event(session, H2_PROXYS_EV_DATA_READ, 0, NULL);
> + }
> + else {
> + dispatch_event(session, H2_PROXYS_EV_CONN_ERROR, status,
> NULL);
> + return status;
> + }
> + }
Hm. I think this does not catch the case where we sent a PING to the backend
and are waiting for the reply because we might have
suspended streams in this case?
So maybe instead of
!has_suspended_streams(session)
we need to check for
(!has_suspended_streams(session) || session->check_ping)
?
> + else if (check_suspended(session) == APR_EAGAIN) {
> /* no stream has become resumed. Do a blocking read with
> * ever increasing timeouts... */
> if (session->wait_timeout < 25) {
Regards
Rüdiger
