You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ic...@apache.org on 2016/01/05 17:56:18 UTC
svn commit: r1723125 - in /httpd/httpd/trunk: CHANGES
modules/http2/h2_conn.c modules/http2/h2_filter.c
modules/http2/h2_session.c modules/http2/h2_session.h
Author: icing
Date: Tue Jan 5 16:56:18 2016
New Revision: 1723125
URL: http://svn.apache.org/viewvc?rev=1723125&view=rev
Log:
no async http2 read for now, fixed GOAWAY flush at end of session, scoreboard keepalive status shown
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/modules/http2/h2_conn.c
httpd/httpd/trunk/modules/http2/h2_filter.c
httpd/httpd/trunk/modules/http2/h2_session.c
httpd/httpd/trunk/modules/http2/h2_session.h
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1723125&r1=1723124&r2=1723125&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Tue Jan 5 16:56:18 2016
@@ -1,19 +1,15 @@
-*- coding: utf-8 -*-
Changes with Apache 2.5.0
+ *) mod_http2: async reading for http2 connections disabled for now. Fixed
+ flushing of last GOAWAY frame. Fixed calculation of last stream id
+ accepted as described in rfc7540. Reading in KEEPALIVE state now
+ correctly show in scoreboard. Fixed possible race in connection
+ shutdown after review by Ylavic. [Stefan Eissing]
+
*) mod_ssl: Avoid one TLS record (application data) fragmentation by
including the last suitable bucket when coalescing. [Yann Ylavic]
- *) mod_http2: reworked synching of session shutdown with worker threads. h2
- workers now stick to a session until no more reuqquest are tbd, keepalive
- handling revisited, users report problems with connection close without
- GOAWAY frames. [Stefan Eissing]
-
- *) mod_http2: Fixed several errors when connections are closed in the middle
- of requests, changed H2KeepAliveTimeout defaults to be the same as H2Timeout
- for synchronous MPMs and leave keepalive timeout to async MPMs default.
- [Stefan Eissing]
-
*) mod_cache_socache: Fix a possible cached entity body corruption when it
is received from an origin server in multiple batches and forwarded by
mod_proxy. [Yann Ylavic]
Modified: httpd/httpd/trunk/modules/http2/h2_conn.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_conn.c?rev=1723125&r1=1723124&r2=1723125&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_conn.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_conn.c Tue Jan 5 16:56:18 2016
@@ -167,9 +167,6 @@ apr_status_t h2_conn_setup(h2_ctx *ctx,
}
h2_ctx_session_set(ctx, session);
-
- ap_update_child_status_from_conn(c->sbh, SERVER_BUSY_READ, c);
-
return APR_SUCCESS;
}
Modified: httpd/httpd/trunk/modules/http2/h2_filter.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_filter.c?rev=1723125&r1=1723124&r2=1723125&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_filter.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_filter.c Tue Jan 5 16:56:18 2016
@@ -133,7 +133,6 @@ apr_status_t h2_filter_core_input(ap_fil
apr_socket_timeout_set(cin->socket, t);
}
}
- ap_update_child_status_from_conn(f->c->sbh, SERVER_BUSY_READ, f->c);
status = ap_get_brigade(f->next, cin->bb, AP_MODE_READBYTES,
block, readbytes);
if (saved_timeout != UNSET) {
Modified: httpd/httpd/trunk/modules/http2/h2_session.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_session.c?rev=1723125&r1=1723124&r2=1723125&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_session.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_session.c Tue Jan 5 16:56:18 2016
@@ -695,6 +695,46 @@ static void h2_session_destroy(h2_sessio
}
}
+static apr_status_t h2_session_shutdown(h2_session *session, int reason)
+{
+ apr_status_t status = APR_SUCCESS;
+
+ AP_DEBUG_ASSERT(session);
+ session->aborted = 1;
+ if (session->state != H2_SESSION_ST_CLOSING
+ && session->state != H2_SESSION_ST_ABORTED) {
+ h2_mplx_abort(session->mplx);
+ if (session->server_goaway) {
+ /* already sent one */
+ }
+ else if (!reason) {
+ nghttp2_submit_goaway(session->ngh2, NGHTTP2_FLAG_NONE,
+ h2_mplx_get_max_stream_started(session->mplx),
+ reason, NULL, 0);
+ status = nghttp2_session_send(session->ngh2);
+ session->server_goaway = 1;
+ ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c,
+ "session(%ld): shutdown, no err", session->id);
+ }
+ else {
+ const char *err = nghttp2_strerror(reason);
+ nghttp2_submit_goaway(session->ngh2, NGHTTP2_FLAG_NONE,
+ h2_mplx_get_max_stream_started(session->mplx),
+ reason, (const uint8_t *)err,
+ strlen(err));
+ status = nghttp2_session_send(session->ngh2);
+ session->server_goaway = 1;
+ ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c,
+ "session(%ld): shutdown, err=%d '%s'",
+ session->id, reason, err);
+ }
+
+ h2_conn_io_flush(&session->io);
+ session->state = H2_SESSION_ST_CLOSING;
+ }
+ return status;
+}
+
static apr_status_t session_pool_cleanup(void *data)
{
h2_session *session = data;
@@ -707,6 +747,22 @@ static apr_status_t session_pool_cleanup
*/
ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, session->c,
"session(%ld): pool_cleanup", session->id);
+
+ AP_DEBUG_ASSERT(session->aborted || session->server_goaway);
+ if (!session->aborted && !session->server_goaway) {
+ /* Not good. The connection is being torn down and we have
+ * not sent a goaway. This is considered a protocol error and
+ * the client has to assume that any streams "in flight" may have
+ * been processed and are not safe to retry.
+ * As clients with idle connection may only learn about a closed
+ * connection when sending the next request, this has the effect
+ * that at least this one request will fail.
+ */
+ ap_log_cerror(APLOG_MARK, APLOG_WARNING, 0, session->c,
+ "session(%ld): connection disappeared without proper "
+ "goodbye, clients will be confused, should not happen",
+ session->id);
+ }
/* keep us from destroying the pool, since that is already ongoing. */
session->pool = NULL;
h2_session_destroy(session);
@@ -889,44 +945,6 @@ void h2_session_eoc_callback(h2_session
h2_session_destroy(session);
}
-static apr_status_t h2_session_shutdown(h2_session *session, int reason)
-{
- apr_status_t status = APR_SUCCESS;
-
- AP_DEBUG_ASSERT(session);
- session->aborted = 1;
- if (session->state != H2_SESSION_ST_CLOSING
- && session->state != H2_SESSION_ST_ABORTED) {
- h2_mplx_abort(session->mplx);
- if (session->server_goaway) {
- /* already sent one */
- }
- else if (!reason) {
- nghttp2_submit_goaway(session->ngh2, NGHTTP2_FLAG_NONE,
- h2_mplx_get_max_stream_started(session->mplx),
- reason, NULL, 0);
- status = nghttp2_session_send(session->ngh2);
- session->server_goaway = 1;
- ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c,
- "session(%ld): shutdown, no err", session->id);
- }
- else {
- const char *err = nghttp2_strerror(reason);
- nghttp2_submit_goaway(session->ngh2, NGHTTP2_FLAG_NONE,
- h2_mplx_get_max_stream_started(session->mplx),
- reason, (const uint8_t *)err,
- strlen(err));
- status = nghttp2_session_send(session->ngh2);
- session->server_goaway = 1;
- ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c,
- "session(%ld): shutdown, err=%d '%s'",
- session->id, reason, err);
- }
- session->state = H2_SESSION_ST_CLOSING;
- }
- return status;
-}
-
void h2_session_abort(h2_session *session, apr_status_t status)
{
AP_DEBUG_ASSERT(session);
@@ -1716,7 +1734,7 @@ apr_status_t h2_session_process(h2_sessi
{
apr_status_t status = APR_SUCCESS;
conn_rec *c = session->c;
- int rv, have_written, have_read, remain_secs;
+ int rv, have_written, have_read;
const char *reason = "";
ap_log_cerror( APLOG_MARK, APLOG_TRACE1, status, c,
@@ -1740,6 +1758,7 @@ apr_status_t h2_session_process(h2_sessi
session->server_goaway = 1;
}
+ ap_update_child_status(c->sbh, SERVER_BUSY_READ, NULL);
status = h2_session_start(session, &rv);
ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, c,
"h2_session(%ld): started on %s:%d", session->id,
@@ -1755,13 +1774,14 @@ apr_status_t h2_session_process(h2_sessi
break;
case H2_SESSION_ST_IDLE_READ:
- h2_filter_cin_timeout_set(session->cin, session->timeout_secs);
- ap_update_child_status(c->sbh, SERVER_BUSY_READ, NULL);
+ h2_filter_cin_timeout_set(session->cin, session->keepalive_secs);
+ ap_update_child_status(c->sbh, SERVER_BUSY_KEEPALIVE, NULL);
status = h2_session_read(session, 1, 10);
if (APR_STATUS_IS_TIMEUP(status)) {
ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, c,
"h2_session(%ld): IDLE -> KEEPALIVE", session->id);
- session->state = H2_SESSION_ST_KEEPALIVE;
+ h2_session_shutdown(session, 0);
+ goto out;
}
else if (status == APR_SUCCESS) {
/* got something, go busy again */
@@ -1777,6 +1797,7 @@ apr_status_t h2_session_process(h2_sessi
case H2_SESSION_ST_BUSY:
if (nghttp2_session_want_read(session->ngh2)) {
+ ap_update_child_status(c->sbh, SERVER_BUSY_READ, NULL);
h2_filter_cin_timeout_set(session->cin, session->timeout_secs);
status = h2_session_read(session, 0, 10);
if (status == APR_SUCCESS) {
@@ -1879,6 +1900,7 @@ apr_status_t h2_session_process(h2_sessi
}
else if (status == APR_TIMEUP) {
if (nghttp2_session_want_read(session->ngh2)) {
+ ap_update_child_status(c->sbh, SERVER_BUSY_READ, NULL);
status = h2_session_read(session, 0, 1);
if (status == APR_SUCCESS) {
/* got something, go busy again */
@@ -1904,57 +1926,9 @@ apr_status_t h2_session_process(h2_sessi
}
break;
- case H2_SESSION_ST_KEEPALIVE:
- /* Our normal H2Timeout has passed and we are considering to
- * extend that with the H2KeepAliveTimeout. */
- remain_secs = session->keepalive_secs - session->timeout_secs;
- if (remain_secs <= 0) {
- /* keepalive is <= normal timeout, close the session */
- reason = "keepalive expired";
- h2_session_shutdown(session, 0);
- goto out;
- }
- session->c->keepalive = AP_CONN_KEEPALIVE;
- ap_update_child_status_from_conn(c->sbh, SERVER_BUSY_KEEPALIVE, c);
-
- if ((apr_time_sec(session->s->keep_alive_timeout) >= remain_secs)
- && async && session->c->cs
- && !session->r) {
- /* Async MPMs are able to handle keep-alive connections without
- * blocking a thread. For this to happen, we need to return from
- * processing, indicating the IO event we are waiting for, and
- * may be called again if the event happens.
- * TODO: this does not properly GOAWAY connections...
- * TODO: This currently does not work on upgraded requests...
- */
- ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, c,
- "h2_session(%ld): async KEEPALIVE -> IDLE_READ", session->id);
- session->state = H2_SESSION_ST_IDLE_READ;
- session->c->cs->state = CONN_STATE_WRITE_COMPLETION;
- reason = "async keepalive";
- status = APR_SUCCESS;
- goto out;
- }
- ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, c,
- "h2_session(%ld): KEEPALIVE read", session->id);
- h2_filter_cin_timeout_set(session->cin, remain_secs);
- status = h2_session_read(session, 1, 1);
- if (APR_STATUS_IS_TIMEUP(status)) {
- reason = "keepalive expired";
- h2_session_shutdown(session, 0);
- goto out;
- }
- else if (status != APR_SUCCESS) {
- reason = "keepalive error";
- goto out;
- }
- ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, c,
- "h2_session(%ld): KEEPALIVE -> BUSY", session->id);
- session->state = H2_SESSION_ST_BUSY;
- break;
-
case H2_SESSION_ST_CLOSING:
if (nghttp2_session_want_write(session->ngh2)) {
+ ap_update_child_status(c->sbh, SERVER_BUSY_READ, NULL);
status = h2_session_send(session);
if (status != APR_SUCCESS) {
reason = "send error";
Modified: httpd/httpd/trunk/modules/http2/h2_session.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_session.h?rev=1723125&r1=1723124&r2=1723125&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_session.h (original)
+++ httpd/httpd/trunk/modules/http2/h2_session.h Tue Jan 5 16:56:18 2016
@@ -58,7 +58,6 @@ typedef enum {
H2_SESSION_ST_IDLE_READ, /* nothing to write, expecting data inc */
H2_SESSION_ST_BUSY, /* read/write without stop */
H2_SESSION_ST_BUSY_WAIT, /* waiting for tasks reporting back */
- H2_SESSION_ST_KEEPALIVE, /* nothing to write, normal timeout passed */
H2_SESSION_ST_CLOSING, /* shuting down */
H2_SESSION_ST_ABORTED, /* client closed connection or sky fall */
} h2_session_state;