Return-Path: X-Original-To: apmail-httpd-cvs-archive@www.apache.org Delivered-To: apmail-httpd-cvs-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id EACD617C42 for ; Mon, 21 Sep 2015 11:00:05 +0000 (UTC) Received: (qmail 87394 invoked by uid 500); 21 Sep 2015 11:00:00 -0000 Delivered-To: apmail-httpd-cvs-archive@httpd.apache.org Received: (qmail 87324 invoked by uid 500); 21 Sep 2015 11:00:00 -0000 Mailing-List: contact cvs-help@httpd.apache.org; run by ezmlm Precedence: bulk Reply-To: dev@httpd.apache.org list-help: list-unsubscribe: List-Post: List-Id: Delivered-To: mailing list cvs@httpd.apache.org Received: (qmail 87296 invoked by uid 99); 21 Sep 2015 11:00:00 -0000 Received: from Unknown (HELO spamd2-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 21 Sep 2015 11:00:00 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd2-us-west.apache.org (ASF Mail Server at spamd2-us-west.apache.org) with ESMTP id 47BE91A2886 for ; Mon, 21 Sep 2015 11:00:00 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd2-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 4.3 X-Spam-Level: **** X-Spam-Status: No, score=4.3 tagged_above=-999 required=6.31 tests=[KAM_ASCII_DIVIDERS=0.8, KAM_LAZY_DOMAIN_SECURITY=1, RDNS_NONE=2.5] autolearn=disabled Received: from mx1-us-east.apache.org ([10.40.0.8]) by localhost (spamd2-us-west.apache.org [10.40.0.9]) (amavisd-new, port 10024) with ESMTP id Xnl2HnjoC3t5 for ; Mon, 21 Sep 2015 10:59:58 +0000 (UTC) Received: from mailrelay1-us-west.apache.org (unknown [209.188.14.139]) by mx1-us-east.apache.org (ASF Mail Server at mx1-us-east.apache.org) with ESMTP id DB35E4414C for ; Mon, 21 Sep 2015 10:59:57 +0000 (UTC) Received: from svn01-us-west.apache.org (svn.apache.org [10.41.0.6]) by mailrelay1-us-west.apache.org (ASF Mail Server at mailrelay1-us-west.apache.org) with ESMTP id 6317DE046A for ; Mon, 21 Sep 2015 10:59:57 +0000 (UTC) Received: from svn01-us-west.apache.org (localhost [127.0.0.1]) by svn01-us-west.apache.org (ASF Mail Server at svn01-us-west.apache.org) with ESMTP id 3D0EB3A01E3 for ; Mon, 21 Sep 2015 10:59:57 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: svn commit: r1704262 - in /httpd/httpd/trunk/modules/http2: h2_conn.c h2_session.c h2_session.h h2_stream.c h2_stream.h Date: Mon, 21 Sep 2015 10:59:56 -0000 To: cvs@httpd.apache.org From: icing@apache.org X-Mailer: svnmailer-1.0.9 Message-Id: <20150921105957.3D0EB3A01E3@svn01-us-west.apache.org> Author: icing Date: Mon Sep 21 10:59:50 2015 New Revision: 1704262 URL: http://svn.apache.org/viewvc?rev=1704262&view=rev Log: no GOAWAYs on connection failures, sending last received stream id back in GOAWAY, no more flush attempts when session has already been aborted Modified: httpd/httpd/trunk/modules/http2/h2_conn.c httpd/httpd/trunk/modules/http2/h2_session.c httpd/httpd/trunk/modules/http2/h2_session.h httpd/httpd/trunk/modules/http2/h2_stream.c httpd/httpd/trunk/modules/http2/h2_stream.h Modified: httpd/httpd/trunk/modules/http2/h2_conn.c URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_conn.c?rev=1704262&r1=1704261&r2=1704262&view=diff ============================================================================== --- httpd/httpd/trunk/modules/http2/h2_conn.c (original) +++ httpd/httpd/trunk/modules/http2/h2_conn.c Mon Sep 21 10:59:50 2015 @@ -288,24 +288,24 @@ apr_status_t h2_session_process(h2_sessi || session->frames_received <= 1)? APR_BLOCK_READ : APR_NONBLOCK_READ); switch (status) { - case APR_SUCCESS: - /* successful read, reset our idle timers */ + case APR_SUCCESS: /* successful read, reset our idle timers */ have_read = 1; wait_micros = 0; break; - case APR_EAGAIN: + case APR_EAGAIN: /* non-blocking read, nothing there */ break; - case APR_EBADF: + case APR_EBADF: /* connection is not there any more */ case APR_EOF: case APR_ECONNABORTED: case APR_ECONNRESET: + case APR_TIMEUP: /* blocked read, timed out */ ap_log_cerror( APLOG_MARK, APLOG_DEBUG, status, session->c, "h2_session(%ld): reading", session->id); h2_session_abort(session, status, 0); break; default: - ap_log_cerror( APLOG_MARK, APLOG_WARNING, status, session->c, + ap_log_cerror( APLOG_MARK, APLOG_INFO, status, session->c, APLOGNO(02950) "h2_session(%ld): error reading, terminating", session->id); Modified: httpd/httpd/trunk/modules/http2/h2_session.c URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_session.c?rev=1704262&r1=1704261&r2=1704262&view=diff ============================================================================== --- httpd/httpd/trunk/modules/http2/h2_session.c (original) +++ httpd/httpd/trunk/modules/http2/h2_session.c Mon Sep 21 10:59:50 2015 @@ -64,6 +64,9 @@ static int stream_open(h2_session *sessi stream = h2_mplx_open_io(session->mplx, stream_id); if (stream) { h2_stream_set_add(session->streams, stream); + if (stream->id > session->max_stream_received) { + session->max_stream_received = stream->id; + } ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c, "h2_session: stream(%ld-%d): opened", @@ -223,10 +226,25 @@ static int on_frame_not_send_cb(nghttp2_ return 0; } -static apr_status_t stream_destroy(h2_session *session, h2_stream *stream) +static apr_status_t stream_destroy(h2_session *session, + h2_stream *stream, + uint32_t error_code) { - ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c, - "h2_stream(%ld-%d): closing", session->id, (int)stream->id); + if (!error_code) { + ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c, + "h2_stream(%ld-%d): handled, closing", + session->id, (int)stream->id); + if (stream->id > session->max_stream_handled) { + session->max_stream_handled = stream->id; + } + } + else { + ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c, + "h2_stream(%ld-%d): closing with err=%d %s", + session->id, (int)stream->id, (int)error_code, + nghttp2_strerror(error_code)); + } + h2_stream_set_remove(session->streams, stream); return h2_mplx_cleanup_stream(session->mplx, stream); } @@ -243,7 +261,7 @@ static int on_stream_close_cb(nghttp2_se } stream = h2_stream_set_get(session->streams, stream_id); if (stream) { - stream_destroy(session, stream); + stream_destroy(session, stream, error_code); } if (error_code) { @@ -655,50 +673,43 @@ void h2_session_destroy(h2_session *sess } } -apr_status_t h2_session_goaway(h2_session *session, apr_status_t reason) -{ - apr_status_t status = APR_SUCCESS; - int rv; - AP_DEBUG_ASSERT(session); - if (session->aborted) { - return APR_EINVAL; - } - - rv = 0; - if (reason == APR_SUCCESS) { - rv = nghttp2_submit_shutdown_notice(session->ngh2); - } - else { - int err = 0; - int last_id = nghttp2_session_get_last_proc_stream_id(session->ngh2); - rv = nghttp2_submit_goaway(session->ngh2, last_id, - NGHTTP2_FLAG_NONE, err, NULL, 0); - } - if (rv != 0) { - status = APR_EGENERAL; - ap_log_cerror(APLOG_MARK, APLOG_ERR, status, session->c, - APLOGNO(02930) "session(%ld): submit goaway: %s", - session->id, nghttp2_strerror(rv)); - } - return status; -} - static apr_status_t h2_session_abort_int(h2_session *session, int reason) { AP_DEBUG_ASSERT(session); if (!session->aborted) { session->aborted = 1; if (session->ngh2) { - if (reason) { - ap_log_cerror(APLOG_MARK, (reason == NGHTTP2_ERR_EOF)? - APLOG_DEBUG : APLOG_INFO, 0, session->c, + + if (!reason) { + nghttp2_submit_goaway(session->ngh2, NGHTTP2_FLAG_NONE, + session->max_stream_received, + reason, NULL, 0); + nghttp2_session_send(session->ngh2); + h2_conn_io_flush(&session->io); + } + else { + const char *err = nghttp2_strerror(reason); + + ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c, "session(%ld): aborting session, reason=%d %s", - session->id, reason, nghttp2_strerror(reason)); + session->id, reason, err); + + if (NGHTTP2_ERR_EOF == reason) { + /* This is our way of indication that the connection is + * gone. No use to send any GOAWAY frames. */ + nghttp2_session_terminate_session(session->ngh2, reason); + } + else { + /* The connection might still be there and we shut down + * with GOAWAY and reason information. */ + nghttp2_submit_goaway(session->ngh2, NGHTTP2_FLAG_NONE, + session->max_stream_received, + reason, (const uint8_t *)err, + strlen(err)); + nghttp2_session_send(session->ngh2); + h2_conn_io_flush(&session->io); + } } - nghttp2_session_terminate_session(session->ngh2, reason); - nghttp2_submit_goaway(session->ngh2, 0, 0, reason, NULL, 0); - nghttp2_session_send(session->ngh2); - h2_conn_io_flush(&session->io); } h2_mplx_abort(session->mplx); } @@ -714,10 +725,12 @@ apr_status_t h2_session_abort(h2_session case APR_ENOMEM: rv = NGHTTP2_ERR_NOMEM; break; - case APR_EOF: - rv = 0; + case APR_SUCCESS: /* all fine, just... */ + case APR_EOF: /* client closed its end... */ + case APR_TIMEUP: /* got bored waiting... */ + rv = 0; /* ...gracefully shut down */ break; - case APR_EBADF: + case APR_EBADF: /* connection unusable, terminate silently */ case APR_ECONNABORTED: rv = NGHTTP2_ERR_EOF; break; @@ -1006,7 +1019,7 @@ apr_status_t h2_session_read(h2_session apr_status_t h2_session_close(h2_session *session) { AP_DEBUG_ASSERT(session); - return h2_conn_io_flush(&session->io); + return session->aborted? APR_SUCCESS : h2_conn_io_flush(&session->io); } /* The session wants to send more DATA for the given stream. Modified: httpd/httpd/trunk/modules/http2/h2_session.h URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_session.h?rev=1704262&r1=1704261&r2=1704262&view=diff ============================================================================== --- httpd/httpd/trunk/modules/http2/h2_session.h (original) +++ httpd/httpd/trunk/modules/http2/h2_session.h Mon Sep 21 10:59:50 2015 @@ -71,44 +71,70 @@ struct h2_session { struct h2_stream_set *streams; /* streams handled by this session */ + int max_stream_received; /* highest stream id created */ + int max_stream_handled; /* highest stream id handled successfully */ + struct nghttp2_session *ngh2; /* the nghttp2 session (internal use) */ struct h2_workers *workers; /* for executing stream tasks */ }; -/* Create a new h2_session for the given connection (mode 'h2'). +/** + * Create a new h2_session for the given connection. * The session will apply the configured parameter. + * @param c the connection to work on + * @param cfg the module config to apply + * @param workers the worker pool to use + * @return the created session */ h2_session *h2_session_create(conn_rec *c, struct h2_config *cfg, struct h2_workers *workers); -/* Create a new h2_session for the given request (mode 'h2c'). +/** + * Create a new h2_session for the given request. * The session will apply the configured parameter. + * @param r the request that was upgraded + * @param cfg the module config to apply + * @param workers the worker pool to use + * @return the created session */ h2_session *h2_session_rcreate(request_rec *r, struct h2_config *cfg, struct h2_workers *workers); -/* Destroy the session and all object it still contains. This will not - * destroy h2_task instances that not finished yet. */ +/** + * Destroy the session and all objects it still contains. This will not + * destroy h2_task instances that have not finished yet. + * @param session the session to destroy + */ void h2_session_destroy(h2_session *session); -/* Called once at start of session. Performs initial client thingies. */ +/** + * Called once at start of session. + * Sets up the session and sends the initial SETTINGS frame. + * @param session the session to start + * @param rv error codes in libnghttp2 lingo are returned here + * @return APR_SUCCESS if all went well + */ apr_status_t h2_session_start(h2_session *session, int *rv); -/* Return != 0 iff session is finished and connection can be closed. +/** + * Determine if session is finished. + * @return != 0 iff session is finished and connection can be closed. */ int h2_session_is_done(h2_session *session); -/* Called when the session will shutdown after all open streams - * are handled. New streams will no longer be accepted. - * Call with reason APR_SUCCESS to initiate a graceful shutdown. */ -apr_status_t h2_session_goaway(h2_session *session, apr_status_t reason); - -/* Called when an error occured and the session needs to shut down. - * Status indicates the reason of the error. */ +/** + * Called when an error occured and the session needs to shut down. + * @param session the session to shut down + * @param reason the apache status that caused the shutdown + * @param rv the nghttp2 reason for shutdown, set to 0 if you have none. + * + */ apr_status_t h2_session_abort(h2_session *session, apr_status_t reason, int rv); -/* Called before a session gets destroyed, might flush output etc. */ +/** + * Called before a session gets destroyed, might flush output etc. + */ apr_status_t h2_session_close(h2_session *session); /* Read more data from the client connection. Used normally with blocking Modified: httpd/httpd/trunk/modules/http2/h2_stream.c URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_stream.c?rev=1704262&r1=1704261&r2=1704262&view=diff ============================================================================== --- httpd/httpd/trunk/modules/http2/h2_stream.c (original) +++ httpd/httpd/trunk/modules/http2/h2_stream.c Mon Sep 21 10:59:50 2015 @@ -61,7 +61,7 @@ h2_stream *h2_stream_create(int id, apr_ return stream; } -void h2_stream_cleanup(h2_stream *stream) +static void h2_stream_cleanup(h2_stream *stream) { if (stream->request) { h2_request_destroy(stream->request); Modified: httpd/httpd/trunk/modules/http2/h2_stream.h URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_stream.h?rev=1704262&r1=1704261&r2=1704262&view=diff ============================================================================== --- httpd/httpd/trunk/modules/http2/h2_stream.h (original) +++ httpd/httpd/trunk/modules/http2/h2_stream.h Mon Sep 21 10:59:50 2015 @@ -72,7 +72,6 @@ struct h2_stream { h2_stream *h2_stream_create(int id, apr_pool_t *pool, struct h2_mplx *m); apr_status_t h2_stream_destroy(h2_stream *stream); -void h2_stream_cleanup(h2_stream *stream); apr_pool_t *h2_stream_detach_pool(h2_stream *stream); void h2_stream_attach_pool(h2_stream *stream, apr_pool_t *pool);