On 01/05/2006 08:59 PM, Ruediger Pluem wrote:
>
> On 01/05/2006 01:51 PM, Ruediger Pluem wrote:
>
> [..cut..]
>
> I finally merged all the commits done to the trunk on this issue
Thanks to Jim for reviewing the patch. He detected one missed patch
and made some comments in the code clearer. The new patch list now:
r354628
r354636
r357461
r357519
r358022 <- new
r365374
r366181
r366554 <- new
r366558 <- new
I created a new merged patch that works with 2.2.x. So the same procedure
again please :).
I would like Brian (Akins) to give it a try and of course if someone else has a
look on it, it wouldn't hurt.
Regards
Rüdiger
Index: modules/http/http_core.c
===================================================================
--- modules/http/http_core.c (Revision 366698)
+++ modules/http/http_core.c (Arbeitskopie)
@@ -39,6 +39,7 @@
AP_DECLARE_DATA ap_filter_rec_t *ap_http_input_filter_handle;
AP_DECLARE_DATA ap_filter_rec_t *ap_http_header_filter_handle;
AP_DECLARE_DATA ap_filter_rec_t *ap_chunk_filter_handle;
+AP_DECLARE_DATA ap_filter_rec_t *ap_http_outerror_filter_handle;
AP_DECLARE_DATA ap_filter_rec_t *ap_byterange_filter_handle;
static const char *set_keep_alive_timeout(cmd_parms *cmd, void *dummy,
@@ -202,6 +203,8 @@
NULL, r, r->connection);
ap_add_output_filter_handle(ap_http_header_filter_handle,
NULL, r, r->connection);
+ ap_add_output_filter_handle(ap_http_outerror_filter_handle,
+ NULL, r, r->connection);
}
return OK;
@@ -237,6 +240,9 @@
ap_chunk_filter_handle =
ap_register_output_filter("CHUNK", ap_http_chunk_filter,
NULL, AP_FTYPE_TRANSCODE);
+ ap_http_outerror_filter_handle =
+ ap_register_output_filter("HTTP_OUTERROR", ap_http_outerror_filter,
+ NULL, AP_FTYPE_PROTOCOL);
ap_byterange_filter_handle =
ap_register_output_filter("BYTERANGE", ap_byterange_filter,
NULL, AP_FTYPE_PROTOCOL);
Index: modules/http/http_filters.c
===================================================================
--- modules/http/http_filters.c (Revision 366698)
+++ modules/http/http_filters.c (Arbeitskopie)
@@ -1318,3 +1318,29 @@
return bufsiz;
}
+/* Filter to handle any error buckets on output */
+apr_status_t ap_http_outerror_filter(ap_filter_t *f,
+ apr_bucket_brigade *b)
+{
+ request_rec *r = f->r;
+ apr_bucket *e;
+
+ for (e = APR_BRIGADE_FIRST(b);
+ e != APR_BRIGADE_SENTINEL(b);
+ e = APR_BUCKET_NEXT(e))
+ {
+ if (AP_BUCKET_IS_ERROR(e)) {
+ /*
+ * Start of error handling state tree. Just one condition
+ * right now :)
+ */
+ if (((ap_bucket_error *)(e->data))->status == HTTP_BAD_GATEWAY) {
+ /* stream aborted and we have not ended it yet */
+ r->connection->keepalive = AP_CONN_CLOSE;
+ }
+ }
+ }
+
+ return ap_pass_brigade(f->next, b);
+}
+
Index: modules/http/mod_core.h
===================================================================
--- modules/http/mod_core.h (Revision 366698)
+++ modules/http/mod_core.h (Arbeitskopie)
@@ -42,6 +42,7 @@
extern AP_DECLARE_DATA ap_filter_rec_t *ap_http_input_filter_handle;
extern AP_DECLARE_DATA ap_filter_rec_t *ap_http_header_filter_handle;
extern AP_DECLARE_DATA ap_filter_rec_t *ap_chunk_filter_handle;
+extern AP_DECLARE_DATA ap_filter_rec_t *ap_http_outerror_filter_handle;
extern AP_DECLARE_DATA ap_filter_rec_t *ap_byterange_filter_handle;
/*
@@ -54,6 +55,10 @@
/* HTTP/1.1 chunked transfer encoding filter. */
apr_status_t ap_http_chunk_filter(ap_filter_t *f, apr_bucket_brigade *b);
+/* Filter to handle any error buckets on output */
+apr_status_t ap_http_outerror_filter(ap_filter_t *f,
+ apr_bucket_brigade *b);
+
char *ap_response_code_string(request_rec *r, int error_index);
/**
Index: modules/http/chunk_filter.c
===================================================================
--- modules/http/chunk_filter.c (Revision 366698)
+++ modules/http/chunk_filter.c (Arbeitskopie)
@@ -39,6 +39,12 @@
#include "mod_core.h"
+/*
+ * A pointer to this is used to memorize in the filter context that a bad
+ * gateway error bucket had been seen. It is used as an invented unique
pointer.
+ */
+static char bad_gateway_seen;
+
apr_status_t ap_http_chunk_filter(ap_filter_t *f, apr_bucket_brigade *b)
{
#define ASCII_CRLF "\015\012"
@@ -67,6 +73,16 @@
eos = e;
break;
}
+ if (AP_BUCKET_IS_ERROR(e)
+ && (((ap_bucket_error *)(e->data))->status
+ == HTTP_BAD_GATEWAY)) {
+ /*
+ * We had a broken backend. Memorize this in the filter
+ * context.
+ */
+ f->ctx = &bad_gateway_seen;
+ continue;
+ }
if (APR_BUCKET_IS_FLUSH(e)) {
flush = e;
more = apr_brigade_split(b, APR_BUCKET_NEXT(e));
@@ -146,13 +162,20 @@
* 2) the trailer
* 3) the end-of-chunked body CRLF
*
- * If there is no EOS bucket, then do nothing.
+ * We only do this if we have not seen an error bucket with
+ * status HTTP_BAD_GATEWAY. We have memorized an
+ * error bucket that we had seen in the filter context.
+ * The error bucket with status HTTP_BAD_GATEWAY indicates that the
+ * connection to the backend (mod_proxy) broke in the middle of the
+ * response. In order to signal the client that something went wrong
+ * we do not create the last-chunk marker and set c->keepalive to
+ * AP_CONN_CLOSE in the core output filter.
*
* XXX: it would be nice to combine this with the end-of-chunk
* marker above, but this is a bit more straight-forward for
* now.
*/
- if (eos != NULL) {
+ if (eos && !f->ctx) {
/* XXX: (2) trailers ... does not yet exist */
e = apr_bucket_immortal_create(ASCII_ZERO ASCII_CRLF
/* <trailers> */
Index: modules/proxy/mod_proxy_ajp.c
===================================================================
--- modules/proxy/mod_proxy_ajp.c (Revision 366698)
+++ modules/proxy/mod_proxy_ajp.c (Arbeitskopie)
@@ -138,6 +138,8 @@
int havebody = 1;
int isok = 1;
apr_off_t bb_len;
+ int data_sent = 0;
+ int rv = 0;
#ifdef FLUSHING_BANDAID
apr_int32_t conn_poll_fd;
apr_pollfd_t *conn_poll;
@@ -348,6 +350,7 @@
"proxy: error processing body");
isok = 0;
}
+ data_sent = 1;
apr_brigade_cleanup(output_brigade);
}
else {
@@ -363,6 +366,7 @@
"proxy: error processing body");
isok = 0;
}
+ data_sent = 1;
break;
default:
isok = 0;
@@ -400,7 +404,11 @@
}
apr_brigade_destroy(input_brigade);
- apr_brigade_destroy(output_brigade);
+ /*
+ * Clear output_brigade to remove possible buckets that remained there
+ * after an error.
+ */
+ apr_brigade_cleanup(output_brigade);
if (status != APR_SUCCESS) {
/* We had a failure: Close connection to backend */
@@ -409,9 +417,38 @@
"proxy: send body failed to %pI (%s)",
conn->worker->cp->addr,
conn->worker->hostname);
- return HTTP_SERVICE_UNAVAILABLE;
+ /*
+ * If we already send data, signal a broken backend connection
+ * upwards in the chain.
+ */
+ if (data_sent) {
+ ap_proxy_backend_broke(r, output_brigade);
+ /* Return DONE to avoid error messages being added to the stream */
+ rv = DONE;
+ } else
+ rv = HTTP_SERVICE_UNAVAILABLE;
}
+ /*
+ * Ensure that we sent an EOS bucket thru the filter chain, if we already
+ * have sent some data. Maybe ap_proxy_backend_broke was called and added
+ * one to the brigade already (no longer making it empty). So we should
+ * not do this in this case.
+ */
+ if (data_sent && !r->eos_sent && APR_BRIGADE_EMPTY(output_brigade)) {
+ e = apr_bucket_eos_create(r->connection->bucket_alloc);
+ APR_BRIGADE_INSERT_TAIL(output_brigade, e);
+ }
+
+ /* If we have added something to the brigade above, sent it */
+ if (!APR_BRIGADE_EMPTY(output_brigade))
+ ap_pass_brigade(r->output_filters, output_brigade);
+
+ apr_brigade_destroy(output_brigade);
+
+ if (rv)
+ return rv;
+
/* Nice we have answer to send to the client */
if (result == CMD_AJP13_END_RESPONSE && isok) {
ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
Index: modules/proxy/proxy_util.c
===================================================================
--- modules/proxy/proxy_util.c (Revision 366698)
+++ modules/proxy/proxy_util.c (Arbeitskopie)
@@ -2126,3 +2126,23 @@
lb_workers_limit = proxy_lb_workers + PROXY_DYNAMIC_BALANCER_LIMIT;
return lb_workers_limit;
}
+
+PROXY_DECLARE(void) ap_proxy_backend_broke(request_rec *r,
+ apr_bucket_brigade *brigade)
+{
+ apr_bucket *e;
+ conn_rec *c = r->connection;
+
+ r->no_cache = 1;
+ /*
+ * If this is a subrequest, then prevent also caching of the main
+ * request.
+ */
+ if (r->main)
+ r->main->no_cache = 1;
+ e = ap_bucket_error_create(HTTP_BAD_GATEWAY, NULL, c->pool,
+ c->bucket_alloc);
+ APR_BRIGADE_INSERT_TAIL(brigade, e);
+ e = apr_bucket_eos_create(c->bucket_alloc);
+ APR_BRIGADE_INSERT_TAIL(brigade, e);
+}
Index: modules/proxy/mod_proxy_http.c
===================================================================
--- modules/proxy/mod_proxy_http.c (Revision 366698)
+++ modules/proxy/mod_proxy_http.c (Arbeitskopie)
@@ -1199,6 +1199,7 @@
* are being read. */
int pread_len = 0;
apr_table_t *save_table;
+ int backend_broke = 0;
bb = apr_brigade_create(p, c->bucket_alloc);
@@ -1480,8 +1481,16 @@
break;
}
else if (rv != APR_SUCCESS) {
+ /* In this case, we are in real trouble because
+ * our backend bailed on us. Pass along a 502 error
+ * error bucket
+ */
ap_log_cerror(APLOG_MARK, APLOG_ERR, rv, c,
"proxy: error reading response");
+ ap_proxy_backend_broke(r, bb);
+ ap_pass_brigade(r->output_filters, bb);
+ backend_broke = 1;
+ backend->close = 1;
break;
}
/* next time try a non-blocking read */
@@ -1547,6 +1556,11 @@
}
} while (interim_response);
+ /* If our connection with the client is to be aborted, return DONE. */
+ if (c->aborted || backend_broke) {
+ return DONE;
+ }
+
if (conf->error_override) {
/* the code above this checks for 'OK' which is what the hook expects
*/
if (ap_is_HTTP_SUCCESS(r->status))
Index: modules/proxy/mod_proxy.h
===================================================================
--- modules/proxy/mod_proxy.h (Revision 366698)
+++ modules/proxy/mod_proxy.h (Arbeitskopie)
@@ -669,6 +669,15 @@
PROXY_DECLARE(int) ap_proxy_connection_create(const char *proxy_function,
proxy_conn_rec *conn,
conn_rec *c, server_rec *s);
+/**
+ * Signal the upstream chain that the connection to the backend broke in the
+ * middle of the response. This is done by sending an error bucket with
+ * status HTTP_BAD_GATEWAY and an EOS bucket up the filter chain.
+ * @param r current request record of client request
+ * @param brigade The brigade that is sent through the output filter chain
+ */
+PROXY_DECLARE(void) ap_proxy_backend_broke(request_rec *r,
+ apr_bucket_brigade *brigade);
/* Scoreboard */
#if MODULE_MAGIC_NUMBER_MAJOR > 20020903
Index: modules/cache/mod_disk_cache.c
===================================================================
--- modules/cache/mod_disk_cache.c (Revision 366698)
+++ modules/cache/mod_disk_cache.c (Arbeitskopie)
@@ -1010,7 +1010,7 @@
* sanity checks.
*/
if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(bb))) {
- if (r->connection->aborted) {
+ if (r->connection->aborted || r->no_cache) {
ap_log_error(APLOG_MARK, APLOG_INFO, 0, r->server,
"disk_cache: Discarding body for URL %s "
"because connection has been aborted.",
