On 01/05/2006 01:51 PM, Ruediger Pluem wrote:
[..cut..]
I finally merged all the commits done to the trunk on this issue
r354628
r354636
r357461
r357519
r365374
r366181
into one patch that works with 2.2.x. From my current point of view all
aspects of this issue should be considered by this patch.
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 357457)
+++ 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_broken_backend_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,
@@ -237,6 +238,10 @@
ap_chunk_filter_handle =
ap_register_output_filter("CHUNK", ap_http_chunk_filter,
NULL, AP_FTYPE_TRANSCODE);
+ ap_broken_backend_filter_handle =
+ ap_register_output_filter("BROKEN_BACKEND",
+ ap_http_broken_backend_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 357457)
+++ modules/http/http_filters.c (Arbeitskopie)
@@ -1043,6 +1043,9 @@
*/
ap_add_output_filter("CHUNK", NULL, r, r->connection);
}
+ /* If we have a Proxy request, add the BROKEN_BACKEND filter now */
+ if (r->proxyreq != PROXYREQ_NONE)
+ ap_add_output_filter("BROKEN_BACKEND", NULL, r, r->connection);
/* Don't remove this filter until after we have added the CHUNK filter.
* Otherwise, f->next won't be the CHUNK filter and thus the first
@@ -1318,3 +1321,23 @@
return bufsiz;
}
+apr_status_t ap_http_broken_backend_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)
+ && (((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 357457)
+++ 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_broken_backend_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 close the connection to the client if backend broke */
+apr_status_t ap_http_broken_backend_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 357457)
+++ 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.
+ * If there is no EOS bucket, or if we had seen an error bucket with
+ * status HTTP_BAD_GATEWAY then do nothing. 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 357457)
+++ 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,37 @@
"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. 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 357457)
+++ 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 357457)
+++ 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,15 @@
break;
}
else if (rv != APR_SUCCESS) {
+ /* In this case, we are in real trouble because
+ * our backend bailed on us.
+ */
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 +1555,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 357457)
+++ 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 357457)
+++ 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.",
