Return-Path: Delivered-To: apmail-httpd-cvs-archive@www.apache.org Received: (qmail 90488 invoked from network); 21 Oct 2005 13:55:13 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (209.237.227.199) by minotaur.apache.org with SMTP; 21 Oct 2005 13:55:12 -0000 Received: (qmail 95043 invoked by uid 500); 21 Oct 2005 13:55:10 -0000 Delivered-To: apmail-httpd-cvs-archive@httpd.apache.org Received: (qmail 95004 invoked by uid 500); 21 Oct 2005 13:55:10 -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 94993 invoked by uid 99); 21 Oct 2005 13:55:09 -0000 X-ASF-Spam-Status: No, hits=-9.4 required=10.0 tests=ALL_TRUSTED,NO_REAL_NAME X-Spam-Check-By: apache.org Received: from [209.237.227.194] (HELO minotaur.apache.org) (209.237.227.194) by apache.org (qpsmtpd/0.29) with SMTP; Fri, 21 Oct 2005 06:55:09 -0700 Received: (qmail 90231 invoked by uid 65534); 21 Oct 2005 13:54:49 -0000 Message-ID: <20051021135449.90228.qmail@minotaur.apache.org> Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r327185 - in /httpd/httpd/trunk: CHANGES modules/proxy/ajp.h modules/proxy/ajp_header.c modules/proxy/ajp_msg.c modules/proxy/mod_proxy_ajp.c Date: Fri, 21 Oct 2005 13:54:46 -0000 To: cvs@httpd.apache.org From: rpluem@apache.org X-Mailer: svnmailer-1.0.5 X-Virus-Checked: Checked by ClamAV on apache.org X-Spam-Rating: minotaur.apache.org 1.6.2 0/1000/N Author: rpluem Date: Fri Oct 21 06:54:38 2005 New Revision: 327185 URL: http://svn.apache.org/viewcvs?rev=327185&view=rev Log: * Fix PR37100 (SEGV in mod_proxy_ajp), by sending the data up the filter chain immediately instead of spooling it completely before passing it to the filter chain. It contains a bandaid to handle intentional flushes from Tomcat side. Further explanation in code and report. ajp.h: Add ajp_msg_reuse prototype mod_proxy_ajp.c: Adjust logic of ap_proxy_ajp_request ajp_msg.c: Add ajp_msg_reuse ajp_header.c: Adjusting logic of ajp_read_header Modified: httpd/httpd/trunk/CHANGES httpd/httpd/trunk/modules/proxy/ajp.h httpd/httpd/trunk/modules/proxy/ajp_header.c httpd/httpd/trunk/modules/proxy/ajp_msg.c httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c Modified: httpd/httpd/trunk/CHANGES URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/CHANGES?rev=327185&r1=327184&r2=327185&view=diff ============================================================================== --- httpd/httpd/trunk/CHANGES [utf-8] (original) +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Oct 21 06:54:38 2005 @@ -2,6 +2,9 @@ Changes with Apache 2.3.0 [Remove entries to the current 2.0 and 2.2 section below, when backported] + *) mod_proxy_ajp: Do not spool the entire response from AJP backend before + sending it up the filter chain. PR37100. [Ruediger Pluem] + *) core: AddOutputFilterByType is ignored for proxied requests. PR31226. [Joe Orton, Ruediger Pluem] Modified: httpd/httpd/trunk/modules/proxy/ajp.h URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/modules/proxy/ajp.h?rev=327185&r1=327184&r2=327185&view=diff ============================================================================== --- httpd/httpd/trunk/modules/proxy/ajp.h (original) +++ httpd/httpd/trunk/modules/proxy/ajp.h Fri Oct 21 06:54:38 2005 @@ -35,6 +35,7 @@ #include "apr_buckets.h" #include "apr_md5.h" #include "apr_network_io.h" +#include "apr_poll.h" #include "apr_pools.h" #include "apr_strings.h" #include "apr_uri.h" @@ -185,6 +186,14 @@ * @return APR_SUCCESS or error */ apr_status_t ajp_msg_reset(ajp_msg_t *msg); + +/** + * Reuse an AJP Message + * + * @param msg AJP Message to reuse + * @return APR_SUCCESS or error + */ +apr_status_t ajp_msg_reuse(ajp_msg_t *msg); /** * Mark the end of an AJP Message Modified: httpd/httpd/trunk/modules/proxy/ajp_header.c URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/modules/proxy/ajp_header.c?rev=327185&r1=327184&r2=327185&view=diff ============================================================================== --- httpd/httpd/trunk/modules/proxy/ajp_header.c (original) +++ httpd/httpd/trunk/modules/proxy/ajp_header.c Fri Oct 21 06:54:38 2005 @@ -615,12 +615,22 @@ { apr_byte_t result; apr_status_t rc; - - rc = ajp_msg_create(r->pool, msg); - if (rc != APR_SUCCESS) { - ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server, - "ajp_read_header: ajp_msg_create failed"); - return rc; + + if (*msg) { + rc = ajp_msg_reuse(*msg); + if (rc != APR_SUCCESS) { + ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server, + "ajp_read_header: ajp_msg_reuse failed"); + return rc; + } + } + else { + rc = ajp_msg_create(r->pool, msg); + if (rc != APR_SUCCESS) { + ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server, + "ajp_read_header: ajp_msg_create failed"); + return rc; + } } ajp_msg_reset(*msg); rc = ajp_ilink_receive(sock, *msg); Modified: httpd/httpd/trunk/modules/proxy/ajp_msg.c URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/modules/proxy/ajp_msg.c?rev=327185&r1=327184&r2=327185&view=diff ============================================================================== --- httpd/httpd/trunk/modules/proxy/ajp_msg.c (original) +++ httpd/httpd/trunk/modules/proxy/ajp_msg.c Fri Oct 21 06:54:38 2005 @@ -139,6 +139,24 @@ } /** + * Reuse an AJP Message + * + * @param msg AJP Message to reuse + * @return APR_SUCCESS or error + */ +apr_status_t ajp_msg_reuse(ajp_msg_t *msg) +{ + apr_byte_t *buf; + + buf = msg->buf; + memset(msg, 0, sizeof(ajp_msg_t)); + msg->buf = buf; + msg->header_len = AJP_HEADER_LEN; + ajp_msg_reset(msg); + return APR_SUCCESS; +} + +/** * Mark the end of an AJP Message * * @param msg AJP Message to end Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c?rev=327185&r1=327184&r2=327185&view=diff ============================================================================== --- httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c (original) +++ httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c Fri Oct 21 06:54:38 2005 @@ -90,6 +90,32 @@ } /* + * XXX: Flushing bandaid + * + * When processing CMD_AJP13_SEND_BODY_CHUNK AJP messages we will do a poll + * with FLUSH_WAIT miliseconds timeout to determine if more data is currently + * available at the backend. If there is no more data available, we flush + * the data to the client by adding a flush bucket to the brigade we pass + * up the filter chain. + * This is only a bandaid to fix the AJP/1.3 protocol shortcoming of not + * sending (actually not having defined) a flush message, when the data + * should be flushed to the client. As soon as this protocol shortcoming is + * fixed this code should be removed. + * + * For further discussion see PR37100. + * http://issues.apache.org/bugzilla/show_bug.cgi?id=37100 + */ +#define FLUSHING_BANDAID 1 + +#ifdef FLUSHING_BANDAID +/* + * Wait 10000 microseconds to find out if more data is currently + * available at the backend. Just an arbitrary choose. + */ +#define FLUSH_WAIT 10000 +#endif + +/* * process the request and write the response. */ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r, @@ -112,6 +138,10 @@ int havebody = 1; int isok = 1; apr_off_t bb_len; +#ifdef FLUSHING_BANDAID + apr_int32_t conn_poll_fd; + apr_pollfd_t *conn_poll; +#endif /* * Send the AJP request to the remote server @@ -134,6 +164,8 @@ /* allocate an AJP message to store the data of the buckets */ status = ajp_alloc_data_msg(r->pool, &buff, &bufsiz, &msg); if (status != APR_SUCCESS) { + /* We had a failure: Close connection to backend */ + conn->close++; ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "proxy: ajp_alloc_data_msg failed"); return HTTP_INTERNAL_SERVER_ERROR; @@ -171,6 +203,8 @@ status = apr_brigade_flatten(input_brigade, buff, &bufsiz); if (status != APR_SUCCESS) { + /* We had a failure: Close connection to backend */ + conn->close++; apr_brigade_destroy(input_brigade); ap_log_error(APLOG_MARK, APLOG_ERR, status, r->server, "proxy: apr_brigade_flatten"); @@ -183,6 +217,8 @@ if (bufsiz > 0) { status = ajp_send_data_msg(conn->sock, msg, bufsiz); if (status != APR_SUCCESS) { + /* We had a failure: Close connection to backend */ + conn->close++; apr_brigade_destroy(input_brigade); ap_log_error(APLOG_MARK, APLOG_ERR, status, r->server, "proxy: send failed to %pI (%s)", @@ -195,9 +231,12 @@ } /* read the response */ + conn->data = NULL; status = ajp_read_header(conn->sock, r, (ajp_msg_t **)&(conn->data)); if (status != APR_SUCCESS) { + /* We had a failure: Close connection to backend */ + conn->close++; apr_brigade_destroy(input_brigade); ap_log_error(APLOG_MARK, APLOG_ERR, status, r->server, "proxy: read response failed from %pI (%s)", @@ -209,6 +248,18 @@ result = ajp_parse_type(r, conn->data); output_brigade = apr_brigade_create(p, r->connection->bucket_alloc); +#ifdef FLUSHING_BANDAID + /* + * Prepare apr_pollfd_t struct for later check if there is currently + * data available from the backend (do not flush response to client) + * or not (flush response to client) + */ + conn_poll = apr_pcalloc(p, sizeof(apr_pollfd_t)); + conn_poll->reqevents = APR_POLLIN; + conn_poll->desc_type = APR_POLL_SOCKET; + conn_poll->desc.s = conn->sock; +#endif + bufsiz = AJP13_MAX_SEND_BODY_SZ; while (isok) { switch (result) { @@ -272,16 +323,42 @@ case CMD_AJP13_SEND_BODY_CHUNK: /* AJP13_SEND_BODY_CHUNK: piece of data */ status = ajp_parse_data(r, conn->data, &size, &buff); - e = apr_bucket_transient_create(buff, size, - r->connection->bucket_alloc); - APR_BRIGADE_INSERT_TAIL(output_brigade, e); - if (status != APR_SUCCESS) + if (status == APR_SUCCESS) { + e = apr_bucket_transient_create(buff, size, + r->connection->bucket_alloc); + APR_BRIGADE_INSERT_TAIL(output_brigade, e); + +#ifdef FLUSHING_BANDAID + /* + * If there is no more data available from backend side + * currently, flush response to client. + */ + if (apr_poll(conn_poll, 1, &conn_poll_fd, FLUSH_WAIT) + == APR_TIMEUP) { + e = apr_bucket_flush_create(r->connection->bucket_alloc); + APR_BRIGADE_INSERT_TAIL(output_brigade, e); + } +#endif + apr_brigade_length(output_brigade, 0, &bb_len); + if (bb_len != -1) + conn->worker->s->read += bb_len; + if (ap_pass_brigade(r->output_filters, + output_brigade) != APR_SUCCESS) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "proxy: error processing body"); + isok = 0; + } + apr_brigade_cleanup(output_brigade); + } + else { isok = 0; + } break; case CMD_AJP13_END_RESPONSE: e = apr_bucket_eos_create(r->connection->bucket_alloc); APR_BRIGADE_INSERT_TAIL(output_brigade, e); - if (ap_pass_brigade(r->output_filters, output_brigade) != APR_SUCCESS) { + if (ap_pass_brigade(r->output_filters, + output_brigade) != APR_SUCCESS) { ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "proxy: error processing body"); isok = 0; @@ -291,6 +368,19 @@ isok = 0; break; } + + /* + * If connection has been aborted by client: Stop working. + * Nevertheless, we regard our operation so far as a success: + * So do not set isok to 0 and set result to CMD_AJP13_END_RESPONSE + * But: Close this connection to the backend. + */ + if (r->connection->aborted) { + conn->close++; + result = CMD_AJP13_END_RESPONSE; + break; + } + if (!isok) break; @@ -310,14 +400,11 @@ } apr_brigade_destroy(input_brigade); - apr_brigade_length(output_brigade, 0, &bb_len); - if (bb_len != -1) - conn->worker->s->read += bb_len; - - if (!isok) - apr_brigade_destroy(output_brigade); + apr_brigade_destroy(output_brigade); if (status != APR_SUCCESS) { + /* We had a failure: Close connection to backend */ + conn->close++; ap_log_error(APLOG_MARK, APLOG_ERR, status, r->server, "proxy: send body failed to %pI (%s)", conn->worker->cp->addr, @@ -340,6 +427,8 @@ conn->worker->cp->addr, conn->worker->hostname); + /* We had a failure: Close connection to backend */ + conn->close++; return HTTP_SERVICE_UNAVAILABLE; }