Passed: apache/httpd#1951 (trunk - 8e54d27)
Build Update for apache/httpd - Build: #1951 Status: Passed Duration: 21 mins and 14 secs Commit: 8e54d27 (trunk) Author: Yann Ylavic Message: No nullglob with ls.. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1893478 13f79535-47bb-0310-9956-ffa450edef68 View the changeset: https://github.com/apache/httpd/compare/c64996059289...8e54d2793c4c View the full build log and details: https://app.travis-ci.com/github/apache/httpd/builds/238072128?utm_medium=notification_source=email -- You can unsubscribe from build emails from the apache/httpd repository going to https://app.travis-ci.com/account/preferences/unsubscribe?repository=16806660_medium=notification_source=email. Or unsubscribe from *all* email updating your settings at https://app.travis-ci.com/account/preferences/unsubscribe?utm_medium=notification_source=email. Or configure specific recipients for build notifications in your .travis.yml file. See https://docs.travis-ci.com/user/notifications.
Errored: apache/httpd#1950 (trunk - c649960)
Build Update for apache/httpd - Build: #1950 Status: Errored Duration: 29 mins and 3 secs Commit: c649960 (trunk) Author: Yann Ylavic Message: mod_example_hooks, mod_optional_fn_export: debug messages at APLOG_DEBUG level Switch from APLOG_NOTICE/ERR to APLOG_DEBUG to avoid filling the logs. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1893477 13f79535-47bb-0310-9956-ffa450edef68 View the changeset: https://github.com/apache/httpd/compare/59a5a8d3e516...c64996059289 View the full build log and details: https://app.travis-ci.com/github/apache/httpd/builds/238071274?utm_medium=notification_source=email -- You can unsubscribe from build emails from the apache/httpd repository going to https://app.travis-ci.com/account/preferences/unsubscribe?repository=16806660_medium=notification_source=email. Or unsubscribe from *all* email updating your settings at https://app.travis-ci.com/account/preferences/unsubscribe?utm_medium=notification_source=email. Or configure specific recipients for build notifications in your .travis.yml file. See https://docs.travis-ci.com/user/notifications.
[Regression in httpd 2.4.49] mod_dav: REPORT requests no longer return errors
Hi,
I think that I have stumbled across a significant regression in httpd 2.4.49
where mod_dav has been changed in a way that makes it ignore the errors
returned by a versioning provider during REPORT requests.
I haven't checked that in detail, but I think that r1892513 [1] is the change
that is causing the new behavior.
Consider the new core part of the dav_method_report() function:
…
result = dav_run_deliver_report(r, resource, doc,
r->output_filters, );
switch (result) {
case OK:
return DONE;
case DECLINED:
/* No one handled the report */
return HTTP_NOT_IMPLEMENTED;
default:
if ((err) != NULL) {
… handle the error
}
Assuming there are no deliver_report hooks, this code is going to call
dav_core_deliver_report(), whose relevant part is as follows:
…
if (vsn_hooks) {
*err = (*vsn_hooks->deliver_report)(r, resource, doc,
r->output_filters);
return OK;
}
…
Here the "return OK" part skips the error handling on the calling site,
even if there is an associated error returned in *err.
In turn, this causes the following regression:
- For a failed request where the server hasn't started sending the response,
it is now going to erroneously send a 200 OK with an empty body instead of
an error response with the appropriate HTTP status.
- For a failed request where the server has started sending the response body,
it is now going to handle it as if it was successfully completed instead of
aborting the connection. The likely outcome of this is that the server is
going to send a truncated payload with a 2xx status indicating success.
This regression can cause unexpected behavior of the Subversion clients,
for example in cases where the (ignored) error occurred due to a non-successful
authorization check. Other DAV clients may be susceptible to some kinds of
unexpected behavior as well.
[1] https://svn.apache.org/r1892513
Thanks,
Evgeny Kotkov
Errored: apache/httpd#1948 (trunk - 59a5a8d)
Build Update for apache/httpd - Build: #1948 Status: Errored Duration: 24 mins and 26 secs Commit: 59a5a8d (trunk) Author: Yann Ylavic Message: More of test -n wants a single argument. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1893476 13f79535-47bb-0310-9956-ffa450edef68 View the changeset: https://github.com/apache/httpd/compare/e711176bde27...59a5a8d3e516 View the full build log and details: https://app.travis-ci.com/github/apache/httpd/builds/238066646?utm_medium=notification_source=email -- You can unsubscribe from build emails from the apache/httpd repository going to https://app.travis-ci.com/account/preferences/unsubscribe?repository=16806660_medium=notification_source=email. Or unsubscribe from *all* email updating your settings at https://app.travis-ci.com/account/preferences/unsubscribe?utm_medium=notification_source=email. Or configure specific recipients for build notifications in your .travis.yml file. See https://docs.travis-ci.com/user/notifications.
Failed: apache/httpd#1947 (trunk - e711176)
Build Update for apache/httpd - Build: #1947 Status: Failed Duration: 17 mins and 58 secs Commit: e711176 (trunk) Author: Yann Ylavic Message: test -n wants a single argument. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1893473 13f79535-47bb-0310-9956-ffa450edef68 View the changeset: https://github.com/apache/httpd/compare/266ae085c342...e711176bde27 View the full build log and details: https://app.travis-ci.com/github/apache/httpd/builds/238056592?utm_medium=notification_source=email -- You can unsubscribe from build emails from the apache/httpd repository going to https://app.travis-ci.com/account/preferences/unsubscribe?repository=16806660_medium=notification_source=email. Or unsubscribe from *all* email updating your settings at https://app.travis-ci.com/account/preferences/unsubscribe?utm_medium=notification_source=email. Or configure specific recipients for build notifications in your .travis.yml file. See https://docs.travis-ci.com/user/notifications.
Errored: apache/httpd#1946 (trunk - 266ae08)
Build Update for apache/httpd - Build: #1946 Status: Errored Duration: 24 mins and 24 secs Commit: 266ae08 (trunk) Author: Yann Ylavic Message: MPMs: cap idle_spawn_rate to MAX_SPAWN_RATE. idle_spawn_rate *= 2 can go above MAX_SPAWN_RATE at some point, and it's not enough for MAX_SPAWN_RATE to be a power of two for MPMs event and worker since idle_spawn_rate is per bucket (num_buckets is not necessarily a power of two). Let's cap on the other MPMs too should MAX_SPAWN_RATE change in the future. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1893471 13f79535-47bb-0310-9956-ffa450edef68 View the changeset: https://github.com/apache/httpd/compare/363472acfd97...266ae085c342 View the full build log and details: https://app.travis-ci.com/github/apache/httpd/builds/238052766?utm_medium=notification_source=email -- You can unsubscribe from build emails from the apache/httpd repository going to https://app.travis-ci.com/account/preferences/unsubscribe?repository=16806660_medium=notification_source=email. Or unsubscribe from *all* email updating your settings at https://app.travis-ci.com/account/preferences/unsubscribe?utm_medium=notification_source=email. Or configure specific recipients for build notifications in your .travis.yml file. See https://docs.travis-ci.com/user/notifications.
Re: svn commit: r1862475 - in /httpd/httpd/trunk: CHANGES modules/http2/h2_conn.c modules/http2/h2_version.h server/mpm/event/event.c
On Mon, Sep 20, 2021 at 12:43 PM ste...@eissing.org wrote:
>
> > Am 20.09.2021 um 12:27 schrieb Ruediger Pluem :
> >
> > On 9/20/21 11:17 AM, ste...@eissing.org wrote:
> >>
> >>> Am 14.09.2021 um 13:43 schrieb Ruediger Pluem :
> >>>
> --- httpd/httpd/trunk/server/mpm/event/event.c (original)
> +++ httpd/httpd/trunk/server/mpm/event/event.c Wed Jul 3 13:46:31 2019
> @@ -1153,10 +1153,11 @@ read_request:
> else if (ap_filter_should_yield(c->output_filters)) {
> pending = OK;
> }
> -if (pending == OK) {
> +if (pending == OK || (pending == DECLINED &&
> + cs->pub.sense == CONN_SENSE_WANT_READ)) {
> /* Still in WRITE_COMPLETION_STATE:
> - * Set a write timeout for this connection, and let the
> - * event thread poll for writeability.
> + * Set a read/write timeout for this connection, and let the
> + * event thread poll for read/writeability.
> */
> cs->queue_timestamp = apr_time_now();
> notify_suspend(cs);
> >>>
> >>> Hm. Showing following code lines from trunk for my next question:
> >>>
> >>> update_reqevents_from_sense(cs, -1);
> >>>
> >>> if cs->pub.sense == CONN_SENSE_WANT_READ we subscribe on to POLLIN only
> >>> on the pfd
> >>>
> >>> apr_thread_mutex_lock(timeout_mutex);
> >>> TO_QUEUE_APPEND(cs->sc->wc_q, cs);
> >>> rv = apr_pollset_add(event_pollset, >pfd);
> >>>
> >>> If the read event triggers we will get back to the
> >>>
> >>> if (cs->pub.state == CONN_STATE_WRITE_COMPLETION) {
> >>> int pending = DECLINED;
> >>>
> >>> above. This means we try flush the output queue if it is not empty and if
> >>> it is empty after this we would fall through to
> >>> set cs->pub.state = CONN_STATE_CHECK_REQUEST_LINE_READABLE; and then add
> >>> the pfd to the pollset again and poll again. This should
> >>> trigger the read event again and we would process the input. But if I see
> >>> this correctly we would need to poll twice for getting
> >>> the read data processed.
For CONN_STATE_WRITE_COMPLETION + CONN_SENSE_WANT_READ we indeed reach
here and will POLLIN, then once readable we come back with
CONN_STATE_WRITE_COMPLETION + CONN_SENSE_DEFAULT so if pending != OK
we'll reach the below:
else if (ap_run_input_pending(c) == OK) {
cs->pub.state = CONN_STATE_READ_REQUEST_LINE;
goto read_request;
}
and thus ap_run_process_connection() directly.
> >>> OTOH if we do not manage to clear the output queue above we would add the
> >>> pfd to the pollset again but this time for writing and
> >>> only once all output has been processed we would take care of the reading.
> >>> Maybe we should take care of both?
> >>
> >> Hmm, but can it? In my understanding CONN_STATE_WRITE_COMPLETION is
> >> intended to flush the out buffers before putting in new work.
> >
> > But a single call dies not need to flush all pending data from my
> > understanding provided that no need to flush the data is found
> > like too much in memory data or too much EOR buckets which mean that we
> > have too much finished requests in the output queue.
> > Hence CONN_STATE_WRITE_COMPLETION could happen multiple times only writing
> > pieces from the data in the output queue.
If pending == OK while we asked for CONN_SENSE_WANT_READ we indeed
lose CONN_SENSE_WANT_READ when coming back here after POLLOUT.
What about this patch (attached)?
I tried to comment a bit the WRITE_COMPLETION_STATE too, because it's
not only about writing...
Cheers;
Yann.
Index: server/mpm/event/event.c
===
--- server/mpm/event/event.c (revision 1893073)
+++ server/mpm/event/event.c (working copy)
@@ -999,7 +1001,7 @@ static void process_socket(apr_thread_t *thd, apr_
{
conn_rec *c;
long conn_id = ID_FROM_CHILD_THREAD(my_child_num, my_thread_num);
-int clogging = 0, from_wc_q = 0;
+int clogging = 0;
apr_status_t rv;
int rc = OK;
@@ -1101,9 +1103,6 @@ read_request:
rc = OK;
}
}
-else if (cs->pub.state == CONN_STATE_WRITE_COMPLETION) {
-from_wc_q = 1;
-}
}
/*
* The process_connection hooks above should set the connection state
@@ -1146,20 +1145,28 @@ read_request:
cs->pub.state = CONN_STATE_LINGER;
}
+/* CONN_STATE_WRITE_COMPLETION is a misnomer, this is actually the
+ * user driver polling state which can be POLLOUT but also POLLIN
+ * depending on cs->pub.sense. But even for CONN_SENSE_WANT_READ
+ * we want the outgoing pipe to be empty before POLLIN, so there
+ * will always be write completion first.
+ */
if (cs->pub.state == CONN_STATE_WRITE_COMPLETION) {
-int pending = DECLINED;
+int pending,
Re: svn commit: r1862475 - in /httpd/httpd/trunk: CHANGES modules/http2/h2_conn.c modules/http2/h2_version.h server/mpm/event/event.c
> Am 20.09.2021 um 12:27 schrieb Ruediger Pluem :
>
>
>
> On 9/20/21 11:17 AM, ste...@eissing.org wrote:
>>
>>
>>> Am 14.09.2021 um 13:43 schrieb Ruediger Pluem :
>>>
>>> When looking at this again while researching something I couldn't answer
>>> myself the questions below
>>> and as event mpm and mod_http2 are sometimes pretty complex I thought I ask
>>> :-)
>>>
>>> On 7/3/19 3:46 PM, ic...@apache.org wrote:
Author: icing
Date: Wed Jul 3 13:46:31 2019
New Revision: 1862475
URL: http://svn.apache.org/viewvc?rev=1862475=rev
Log:
*) mod_http2/mpm_event: Fixes the behaviour when a HTTP/2 connection has
nothing
more to write with streams ongoing (flow control block). The timeout
waiting
for the client to send WINODW_UPDATE was incorrectly KeepAliveTimeout
and not
Timeout as it should be. Fixes PR 63534. [Yann Ylavic, Stefan Eissing]
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/modules/http2/h2_conn.c
httpd/httpd/trunk/modules/http2/h2_version.h
httpd/httpd/trunk/server/mpm/event/event.c
>>>
Modified: httpd/httpd/trunk/modules/http2/h2_conn.c
URL:
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_conn.c?rev=1862475=1862474=1862475=diff
==
--- httpd/httpd/trunk/modules/http2/h2_conn.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_conn.c Wed Jul 3 13:46:31 2019
@@ -231,6 +231,13 @@ apr_status_t h2_conn_run(conn_rec *c)
case H2_SESSION_ST_BUSY:
case H2_SESSION_ST_WAIT:
c->cs->state = CONN_STATE_WRITE_COMPLETION;
+if (c->cs && (session->open_streams ||
!session->remote.emitted_count)) {
+/* let the MPM know that we are not done and want
+ * the Timeout behaviour instead of a KeepAliveTimeout
+ * See PR 63534.
+ */
+c->cs->sense = CONN_SENSE_WANT_READ;
>>>
>>> Can we get here with session->open_streams > 0 and all the open streams are
>>> only producing output and none of them is waiting for
>>> something from the client?
>>
>> Yes, but only when the HTTP/2 flow control blocks any progress. New data has
>> to arrive from the client before the server is able to send more. So, we are
>> waiting for new data from the client in the form of window updates.
>
> Thanks, but as you state in this case it wants to read data, not for a new
> request for getting output data sending for existing
> requests started again. This sounds fine. I was worried that we could wait
> for input data here without expecting any.
>
>>
+}
break;
case H2_SESSION_ST_CLEANUP:
case H2_SESSION_ST_DONE:
>>>
Modified: httpd/httpd/trunk/server/mpm/event/event.c
URL:
http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/event/event.c?rev=1862475=1862474=1862475=diff
==
--- httpd/httpd/trunk/server/mpm/event/event.c (original)
+++ httpd/httpd/trunk/server/mpm/event/event.c Wed Jul 3 13:46:31 2019
@@ -1153,10 +1153,11 @@ read_request:
else if (ap_filter_should_yield(c->output_filters)) {
pending = OK;
}
-if (pending == OK) {
+if (pending == OK || (pending == DECLINED &&
+ cs->pub.sense == CONN_SENSE_WANT_READ)) {
/* Still in WRITE_COMPLETION_STATE:
- * Set a write timeout for this connection, and let the
- * event thread poll for writeability.
+ * Set a read/write timeout for this connection, and let the
+ * event thread poll for read/writeability.
*/
cs->queue_timestamp = apr_time_now();
notify_suspend(cs);
>>>
>>> Hm. Showing following code lines from trunk for my next question:
>>>
>>>
>>> update_reqevents_from_sense(cs, -1);
>>>
>>> if cs->pub.sense == CONN_SENSE_WANT_READ we subscribe on to POLLIN only on
>>> the pfd
>>>
>>>
>>> apr_thread_mutex_lock(timeout_mutex);
>>> TO_QUEUE_APPEND(cs->sc->wc_q, cs);
>>> rv = apr_pollset_add(event_pollset, >pfd);
>>>
>>> If the read event triggers we will get back to the
>>>
>>> if (cs->pub.state == CONN_STATE_WRITE_COMPLETION) {
>>> int pending = DECLINED;
>>>
>>> above. This means we try flush the output queue if it is not empty and if
>>> it is empty after this we would fall through to
>>> set cs->pub.state = CONN_STATE_CHECK_REQUEST_LINE_READABLE; and then add
>>> the pfd to the pollset again and poll again. This should
>>> trigger the
Re: svn commit: r1862475 - in /httpd/httpd/trunk: CHANGES modules/http2/h2_conn.c modules/http2/h2_version.h server/mpm/event/event.c
On 9/20/21 11:17 AM, ste...@eissing.org wrote:
>
>
>> Am 14.09.2021 um 13:43 schrieb Ruediger Pluem :
>>
>> When looking at this again while researching something I couldn't answer
>> myself the questions below
>> and as event mpm and mod_http2 are sometimes pretty complex I thought I ask
>> :-)
>>
>> On 7/3/19 3:46 PM, ic...@apache.org wrote:
>>> Author: icing
>>> Date: Wed Jul 3 13:46:31 2019
>>> New Revision: 1862475
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1862475=rev
>>> Log:
>>> *) mod_http2/mpm_event: Fixes the behaviour when a HTTP/2 connection has
>>> nothing
>>> more to write with streams ongoing (flow control block). The timeout
>>> waiting
>>> for the client to send WINODW_UPDATE was incorrectly KeepAliveTimeout
>>> and not
>>> Timeout as it should be. Fixes PR 63534. [Yann Ylavic, Stefan Eissing]
>>>
>>>
>>> Modified:
>>>httpd/httpd/trunk/CHANGES
>>>httpd/httpd/trunk/modules/http2/h2_conn.c
>>>httpd/httpd/trunk/modules/http2/h2_version.h
>>>httpd/httpd/trunk/server/mpm/event/event.c
>>>
>>
>>> Modified: httpd/httpd/trunk/modules/http2/h2_conn.c
>>> URL:
>>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_conn.c?rev=1862475=1862474=1862475=diff
>>> ==
>>> --- httpd/httpd/trunk/modules/http2/h2_conn.c (original)
>>> +++ httpd/httpd/trunk/modules/http2/h2_conn.c Wed Jul 3 13:46:31 2019
>>> @@ -231,6 +231,13 @@ apr_status_t h2_conn_run(conn_rec *c)
>>> case H2_SESSION_ST_BUSY:
>>> case H2_SESSION_ST_WAIT:
>>> c->cs->state = CONN_STATE_WRITE_COMPLETION;
>>> +if (c->cs && (session->open_streams ||
>>> !session->remote.emitted_count)) {
>>> +/* let the MPM know that we are not done and want
>>> + * the Timeout behaviour instead of a KeepAliveTimeout
>>> + * See PR 63534.
>>> + */
>>> +c->cs->sense = CONN_SENSE_WANT_READ;
>>
>> Can we get here with session->open_streams > 0 and all the open streams are
>> only producing output and none of them is waiting for
>> something from the client?
>
> Yes, but only when the HTTP/2 flow control blocks any progress. New data has
> to arrive from the client before the server is able to send more. So, we are
> waiting for new data from the client in the form of window updates.
Thanks, but as you state in this case it wants to read data, not for a new
request for getting output data sending for existing
requests started again. This sounds fine. I was worried that we could wait for
input data here without expecting any.
>
>>> +}
>>> break;
>>> case H2_SESSION_ST_CLEANUP:
>>> case H2_SESSION_ST_DONE:
>>>
>>
>>> Modified: httpd/httpd/trunk/server/mpm/event/event.c
>>> URL:
>>> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/event/event.c?rev=1862475=1862474=1862475=diff
>>> ==
>>> --- httpd/httpd/trunk/server/mpm/event/event.c (original)
>>> +++ httpd/httpd/trunk/server/mpm/event/event.c Wed Jul 3 13:46:31 2019
>>> @@ -1153,10 +1153,11 @@ read_request:
>>> else if (ap_filter_should_yield(c->output_filters)) {
>>> pending = OK;
>>> }
>>> -if (pending == OK) {
>>> +if (pending == OK || (pending == DECLINED &&
>>> + cs->pub.sense == CONN_SENSE_WANT_READ)) {
>>> /* Still in WRITE_COMPLETION_STATE:
>>> - * Set a write timeout for this connection, and let the
>>> - * event thread poll for writeability.
>>> + * Set a read/write timeout for this connection, and let the
>>> + * event thread poll for read/writeability.
>>> */
>>> cs->queue_timestamp = apr_time_now();
>>> notify_suspend(cs);
>>
>> Hm. Showing following code lines from trunk for my next question:
>>
>>
>>update_reqevents_from_sense(cs, -1);
>>
>> if cs->pub.sense == CONN_SENSE_WANT_READ we subscribe on to POLLIN only on
>> the pfd
>>
>>
>>apr_thread_mutex_lock(timeout_mutex);
>>TO_QUEUE_APPEND(cs->sc->wc_q, cs);
>>rv = apr_pollset_add(event_pollset, >pfd);
>>
>> If the read event triggers we will get back to the
>>
>>if (cs->pub.state == CONN_STATE_WRITE_COMPLETION) {
>>int pending = DECLINED;
>>
>> above. This means we try flush the output queue if it is not empty and if it
>> is empty after this we would fall through to
>> set cs->pub.state = CONN_STATE_CHECK_REQUEST_LINE_READABLE; and then add the
>> pfd to the pollset again and poll again. This should
>> trigger the read event again and we would process the input. But if I see
>> this correctly we would need to poll twice for getting
>> the read data processed.
>> OTOH if we do not manage
Re: svn commit: r1862475 - in /httpd/httpd/trunk: CHANGES modules/http2/h2_conn.c modules/http2/h2_version.h server/mpm/event/event.c
> Am 20.09.2021 um 11:17 schrieb ste...@eissing.org:
>
>
>
>> Am 14.09.2021 um 13:43 schrieb Ruediger Pluem :
>>
>> When looking at this again while researching something I couldn't answer
>> myself the questions below
>> and as event mpm and mod_http2 are sometimes pretty complex I thought I ask
>> :-)
>>
>> On 7/3/19 3:46 PM, ic...@apache.org wrote:
>>> Author: icing
>>> Date: Wed Jul 3 13:46:31 2019
>>> New Revision: 1862475
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1862475=rev
>>> Log:
>>> *) mod_http2/mpm_event: Fixes the behaviour when a HTTP/2 connection has
>>> nothing
>>>more to write with streams ongoing (flow control block). The timeout
>>> waiting
>>>for the client to send WINODW_UPDATE was incorrectly KeepAliveTimeout
>>> and not
>>>Timeout as it should be. Fixes PR 63534. [Yann Ylavic, Stefan Eissing]
>>>
>>>
>>> Modified:
>>> httpd/httpd/trunk/CHANGES
>>> httpd/httpd/trunk/modules/http2/h2_conn.c
>>> httpd/httpd/trunk/modules/http2/h2_version.h
>>> httpd/httpd/trunk/server/mpm/event/event.c
>>>
>>
>>> Modified: httpd/httpd/trunk/modules/http2/h2_conn.c
>>> URL:
>>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_conn.c?rev=1862475=1862474=1862475=diff
>>> ==
>>> --- httpd/httpd/trunk/modules/http2/h2_conn.c (original)
>>> +++ httpd/httpd/trunk/modules/http2/h2_conn.c Wed Jul 3 13:46:31 2019
>>> @@ -231,6 +231,13 @@ apr_status_t h2_conn_run(conn_rec *c)
>>>case H2_SESSION_ST_BUSY:
>>>case H2_SESSION_ST_WAIT:
>>>c->cs->state = CONN_STATE_WRITE_COMPLETION;
>>> +if (c->cs && (session->open_streams ||
>>> !session->remote.emitted_count)) {
>>> +/* let the MPM know that we are not done and want
>>> + * the Timeout behaviour instead of a KeepAliveTimeout
>>> + * See PR 63534.
>>> + */
>>> +c->cs->sense = CONN_SENSE_WANT_READ;
>>
>> Can we get here with session->open_streams > 0 and all the open streams are
>> only producing output and none of them is waiting for
>> something from the client?
>
> Yes, but only when the HTTP/2 flow control blocks any progress. New data has
> to arrive from the client before the server is able to send more. So, we are
> waiting for new data from the client in the form of window updates.
>
>>> +}
>>>break;
>>>case H2_SESSION_ST_CLEANUP:
>>>case H2_SESSION_ST_DONE:
>>>
>>
>>> Modified: httpd/httpd/trunk/server/mpm/event/event.c
>>> URL:
>>> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/event/event.c?rev=1862475=1862474=1862475=diff
>>> ==
>>> --- httpd/httpd/trunk/server/mpm/event/event.c (original)
>>> +++ httpd/httpd/trunk/server/mpm/event/event.c Wed Jul 3 13:46:31 2019
>>> @@ -1153,10 +1153,11 @@ read_request:
>>>else if (ap_filter_should_yield(c->output_filters)) {
>>>pending = OK;
>>>}
>>> -if (pending == OK) {
>>> +if (pending == OK || (pending == DECLINED &&
>>> + cs->pub.sense == CONN_SENSE_WANT_READ)) {
>>>/* Still in WRITE_COMPLETION_STATE:
>>> - * Set a write timeout for this connection, and let the
>>> - * event thread poll for writeability.
>>> + * Set a read/write timeout for this connection, and let the
>>> + * event thread poll for read/writeability.
>>> */
>>>cs->queue_timestamp = apr_time_now();
>>>notify_suspend(cs);
>>
>> Hm. Showing following code lines from trunk for my next question:
>>
>>
>> update_reqevents_from_sense(cs, -1);
>>
>> if cs->pub.sense == CONN_SENSE_WANT_READ we subscribe on to POLLIN only on
>> the pfd
>>
>>
>> apr_thread_mutex_lock(timeout_mutex);
>> TO_QUEUE_APPEND(cs->sc->wc_q, cs);
>> rv = apr_pollset_add(event_pollset, >pfd);
>>
>> If the read event triggers we will get back to the
>>
>> if (cs->pub.state == CONN_STATE_WRITE_COMPLETION) {
>> int pending = DECLINED;
>>
>> above. This means we try flush the output queue if it is not empty and if it
>> is empty after this we would fall through to
>> set cs->pub.state = CONN_STATE_CHECK_REQUEST_LINE_READABLE; and then add the
>> pfd to the pollset again and poll again. This should
>> trigger the read event again and we would process the input. But if I see
>> this correctly we would need to poll twice for getting
>> the read data processed.
>> OTOH if we do not manage to clear the output queue above we would add the
>> pfd to the pollset again but this time for writing and
>> only once all output has been processed we would take care of the reading.
>> Maybe we should take care of both?
>
> Hmm, but can it? In
Re: svn commit: r1862475 - in /httpd/httpd/trunk: CHANGES modules/http2/h2_conn.c modules/http2/h2_version.h server/mpm/event/event.c
> Am 14.09.2021 um 13:43 schrieb Ruediger Pluem :
>
> When looking at this again while researching something I couldn't answer
> myself the questions below
> and as event mpm and mod_http2 are sometimes pretty complex I thought I ask
> :-)
>
> On 7/3/19 3:46 PM, ic...@apache.org wrote:
>> Author: icing
>> Date: Wed Jul 3 13:46:31 2019
>> New Revision: 1862475
>>
>> URL: http://svn.apache.org/viewvc?rev=1862475=rev
>> Log:
>> *) mod_http2/mpm_event: Fixes the behaviour when a HTTP/2 connection has
>> nothing
>> more to write with streams ongoing (flow control block). The timeout
>> waiting
>> for the client to send WINODW_UPDATE was incorrectly KeepAliveTimeout
>> and not
>> Timeout as it should be. Fixes PR 63534. [Yann Ylavic, Stefan Eissing]
>>
>>
>> Modified:
>>httpd/httpd/trunk/CHANGES
>>httpd/httpd/trunk/modules/http2/h2_conn.c
>>httpd/httpd/trunk/modules/http2/h2_version.h
>>httpd/httpd/trunk/server/mpm/event/event.c
>>
>
>> Modified: httpd/httpd/trunk/modules/http2/h2_conn.c
>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_conn.c?rev=1862475=1862474=1862475=diff
>> ==
>> --- httpd/httpd/trunk/modules/http2/h2_conn.c (original)
>> +++ httpd/httpd/trunk/modules/http2/h2_conn.c Wed Jul 3 13:46:31 2019
>> @@ -231,6 +231,13 @@ apr_status_t h2_conn_run(conn_rec *c)
>> case H2_SESSION_ST_BUSY:
>> case H2_SESSION_ST_WAIT:
>> c->cs->state = CONN_STATE_WRITE_COMPLETION;
>> +if (c->cs && (session->open_streams ||
>> !session->remote.emitted_count)) {
>> +/* let the MPM know that we are not done and want
>> + * the Timeout behaviour instead of a KeepAliveTimeout
>> + * See PR 63534.
>> + */
>> +c->cs->sense = CONN_SENSE_WANT_READ;
>
> Can we get here with session->open_streams > 0 and all the open streams are
> only producing output and none of them is waiting for
> something from the client?
Yes, but only when the HTTP/2 flow control blocks any progress. New data has to
arrive from the client before the server is able to send more. So, we are
waiting for new data from the client in the form of window updates.
>> +}
>> break;
>> case H2_SESSION_ST_CLEANUP:
>> case H2_SESSION_ST_DONE:
>>
>
>> Modified: httpd/httpd/trunk/server/mpm/event/event.c
>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/event/event.c?rev=1862475=1862474=1862475=diff
>> ==
>> --- httpd/httpd/trunk/server/mpm/event/event.c (original)
>> +++ httpd/httpd/trunk/server/mpm/event/event.c Wed Jul 3 13:46:31 2019
>> @@ -1153,10 +1153,11 @@ read_request:
>> else if (ap_filter_should_yield(c->output_filters)) {
>> pending = OK;
>> }
>> -if (pending == OK) {
>> +if (pending == OK || (pending == DECLINED &&
>> + cs->pub.sense == CONN_SENSE_WANT_READ)) {
>> /* Still in WRITE_COMPLETION_STATE:
>> - * Set a write timeout for this connection, and let the
>> - * event thread poll for writeability.
>> + * Set a read/write timeout for this connection, and let the
>> + * event thread poll for read/writeability.
>> */
>> cs->queue_timestamp = apr_time_now();
>> notify_suspend(cs);
>
> Hm. Showing following code lines from trunk for my next question:
>
>
>update_reqevents_from_sense(cs, -1);
>
> if cs->pub.sense == CONN_SENSE_WANT_READ we subscribe on to POLLIN only on
> the pfd
>
>
>apr_thread_mutex_lock(timeout_mutex);
>TO_QUEUE_APPEND(cs->sc->wc_q, cs);
>rv = apr_pollset_add(event_pollset, >pfd);
>
> If the read event triggers we will get back to the
>
>if (cs->pub.state == CONN_STATE_WRITE_COMPLETION) {
>int pending = DECLINED;
>
> above. This means we try flush the output queue if it is not empty and if it
> is empty after this we would fall through to
> set cs->pub.state = CONN_STATE_CHECK_REQUEST_LINE_READABLE; and then add the
> pfd to the pollset again and poll again. This should
> trigger the read event again and we would process the input. But if I see
> this correctly we would need to poll twice for getting
> the read data processed.
> OTOH if we do not manage to clear the output queue above we would add the pfd
> to the pollset again but this time for writing and
> only once all output has been processed we would take care of the reading.
> Maybe we should take care of both?
Hmm, but can it? In my understanding CONN_STATE_WRITE_COMPLETION is intended to
flush the out buffers before putting in new work.
Since conn_rec's are limited to be processes
Re: svn commit: r1862475 - in /httpd/httpd/trunk: CHANGES modules/http2/h2_conn.c modules/http2/h2_version.h server/mpm/event/event.c
Things have been quite busy here, because of the release. Hence I think it's
worth asking again :-).
Regards
Rüdiger
On 9/14/21 1:43 PM, Ruediger Pluem wrote:
> When looking at this again while researching something I couldn't answer
> myself the questions below
> and as event mpm and mod_http2 are sometimes pretty complex I thought I ask
> :-)
>
> On 7/3/19 3:46 PM, ic...@apache.org wrote:
>> Author: icing
>> Date: Wed Jul 3 13:46:31 2019
>> New Revision: 1862475
>>
>> URL: http://svn.apache.org/viewvc?rev=1862475=rev
>> Log:
>> *) mod_http2/mpm_event: Fixes the behaviour when a HTTP/2 connection has
>> nothing
>> more to write with streams ongoing (flow control block). The timeout
>> waiting
>> for the client to send WINODW_UPDATE was incorrectly KeepAliveTimeout
>> and not
>> Timeout as it should be. Fixes PR 63534. [Yann Ylavic, Stefan Eissing]
>>
>>
>> Modified:
>> httpd/httpd/trunk/CHANGES
>> httpd/httpd/trunk/modules/http2/h2_conn.c
>> httpd/httpd/trunk/modules/http2/h2_version.h
>> httpd/httpd/trunk/server/mpm/event/event.c
>>
>
>> Modified: httpd/httpd/trunk/modules/http2/h2_conn.c
>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_conn.c?rev=1862475=1862474=1862475=diff
>> ==
>> --- httpd/httpd/trunk/modules/http2/h2_conn.c (original)
>> +++ httpd/httpd/trunk/modules/http2/h2_conn.c Wed Jul 3 13:46:31 2019
>> @@ -231,6 +231,13 @@ apr_status_t h2_conn_run(conn_rec *c)
>> case H2_SESSION_ST_BUSY:
>> case H2_SESSION_ST_WAIT:
>> c->cs->state = CONN_STATE_WRITE_COMPLETION;
>> +if (c->cs && (session->open_streams ||
>> !session->remote.emitted_count)) {
>> +/* let the MPM know that we are not done and want
>> + * the Timeout behaviour instead of a KeepAliveTimeout
>> + * See PR 63534.
>> + */
>> +c->cs->sense = CONN_SENSE_WANT_READ;
>
> Can we get here with session->open_streams > 0 and all the open streams are
> only producing output and none of them is waiting for
> something from the client?
>
>
>> +}
>> break;
>> case H2_SESSION_ST_CLEANUP:
>> case H2_SESSION_ST_DONE:
>>
>
>> Modified: httpd/httpd/trunk/server/mpm/event/event.c
>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/event/event.c?rev=1862475=1862474=1862475=diff
>> ==
>> --- httpd/httpd/trunk/server/mpm/event/event.c (original)
>> +++ httpd/httpd/trunk/server/mpm/event/event.c Wed Jul 3 13:46:31 2019
>> @@ -1153,10 +1153,11 @@ read_request:
>> else if (ap_filter_should_yield(c->output_filters)) {
>> pending = OK;
>> }
>> -if (pending == OK) {
>> +if (pending == OK || (pending == DECLINED &&
>> + cs->pub.sense == CONN_SENSE_WANT_READ)) {
>> /* Still in WRITE_COMPLETION_STATE:
>> - * Set a write timeout for this connection, and let the
>> - * event thread poll for writeability.
>> + * Set a read/write timeout for this connection, and let the
>> + * event thread poll for read/writeability.
>> */
>> cs->queue_timestamp = apr_time_now();
>> notify_suspend(cs);
>
> Hm. Showing following code lines from trunk for my next question:
>
>
> update_reqevents_from_sense(cs, -1);
>
> if cs->pub.sense == CONN_SENSE_WANT_READ we subscribe on to POLLIN only on
> the pfd
>
>
> apr_thread_mutex_lock(timeout_mutex);
> TO_QUEUE_APPEND(cs->sc->wc_q, cs);
> rv = apr_pollset_add(event_pollset, >pfd);
>
> If the read event triggers we will get back to the
>
> if (cs->pub.state == CONN_STATE_WRITE_COMPLETION) {
> int pending = DECLINED;
>
> above. This means we try flush the output queue if it is not empty and if it
> is empty after this we would fall through to
> set cs->pub.state = CONN_STATE_CHECK_REQUEST_LINE_READABLE; and then add the
> pfd to the pollset again and poll again. This should
> trigger the read event again and we would process the input. But if I see
> this correctly we would need to poll twice for getting
> the read data processed.
> OTOH if we do not manage to clear the output queue above we would add the pfd
> to the pollset again but this time for writing and
> only once all output has been processed we would take care of the reading.
> Maybe we should take care of both?
>
>
> Regards
>
> Rüdiger
>
>
