On 12/16/2005 09:41 AM, Plüm wrote:
[..cut..]
>
> Currently I am away from my development env. I hope I can post
> a complete patch with all my ideas by tomorrow.
>
I worked out a new version of the patch. It is attached. I checked
it again with my jsp and it seems to work well. If nobody has further
objections I would commit to the trunk. After that I would do the following
next steps:
- Examine mod_proxy_ajp for the same problem.
- Put the common code of mod_proxy_ajp and mod_proxy_http to signal a broken
connection in a new function in proxy_util
Regards
Rüdiger
Index: server/core_filters.c
===================================================================
--- server/core_filters.c (Revision 357328)
+++ server/core_filters.c (Arbeitskopie)
@@ -315,8 +315,10 @@
apr_size_t *bytes_written,
conn_rec *c);
-static void remove_empty_buckets(apr_bucket_brigade *bb);
+static void detect_error_bucket(apr_bucket *bucket, conn_rec *c);
+static void remove_empty_buckets(apr_bucket_brigade *bb, conn_rec *c);
+
static apr_status_t send_brigade_blocking(apr_socket_t *s,
apr_bucket_brigade *bb,
apr_size_t *bytes_written,
@@ -487,7 +489,7 @@
if (bb == NULL) {
return;
}
- remove_empty_buckets(bb);
+ remove_empty_buckets(bb, c);
if (!APR_BRIGADE_EMPTY(bb)) {
c->data_in_output_filters = 1;
if (make_a_copy) {
@@ -526,7 +528,7 @@
struct iovec vec[MAX_IOVEC_TO_WRITE];
apr_size_t nvec = 0;
- remove_empty_buckets(bb);
+ remove_empty_buckets(bb, c);
for (bucket = APR_BRIGADE_FIRST(bb);
bucket != APR_BRIGADE_SENTINEL(bb);
@@ -596,16 +598,26 @@
}
}
- remove_empty_buckets(bb);
+ remove_empty_buckets(bb, c);
return APR_SUCCESS;
}
-static void remove_empty_buckets(apr_bucket_brigade *bb)
+static void detect_error_bucket(apr_bucket *bucket, conn_rec *c)
{
+ if (AP_BUCKET_IS_ERROR(bucket)
+ && (((ap_bucket_error *)(bucket->data))->status == HTTP_BAD_GATEWAY)) {
+ /* stream aborted and we have not ended it yet */
+ c->keepalive = AP_CONN_CLOSE;
+ }
+}
+
+static void remove_empty_buckets(apr_bucket_brigade *bb, conn_rec *c)
+{
apr_bucket *bucket;
while (((bucket = APR_BRIGADE_FIRST(bb)) != APR_BRIGADE_SENTINEL(bb)) &&
(APR_BUCKET_IS_METADATA(bucket) || (bucket->length == 0))) {
+ detect_error_bucket(bucket, c);
APR_BUCKET_REMOVE(bucket);
apr_bucket_destroy(bucket);
}
@@ -678,6 +690,7 @@
for (i = offset; i < nvec; ) {
apr_bucket *bucket = APR_BRIGADE_FIRST(bb);
if (APR_BUCKET_IS_METADATA(bucket)) {
+ detect_error_bucket(bucket, c);
APR_BUCKET_REMOVE(bucket);
apr_bucket_destroy(bucket);
}
Index: modules/http/chunk_filter.c
===================================================================
--- modules/http/chunk_filter.c (Revision 357328)
+++ modules/http/chunk_filter.c (Arbeitskopie)
@@ -47,6 +47,7 @@
apr_bucket_brigade *more;
apr_bucket *e;
apr_status_t rv;
+ int bad_gateway_seen = 0;
for (more = NULL; b; b = more, more = NULL) {
apr_off_t bytes = 0;
@@ -67,6 +68,13 @@
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. */
+ bad_gateway_seen = 1;
+ continue;
+ }
if (APR_BUCKET_IS_FLUSH(e)) {
flush = e;
more = apr_brigade_split(b, APR_BUCKET_NEXT(e));
@@ -146,13 +154,19 @@
* 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.
+ * 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 && !bad_gateway_seen) {
/* XXX: (2) trailers ... does not yet exist */
e = apr_bucket_immortal_create(ASCII_ZERO ASCII_CRLF
/* <trailers> */
Index: modules/proxy/mod_proxy_http.c
===================================================================
--- modules/proxy/mod_proxy_http.c (Revision 357328)
+++ modules/proxy/mod_proxy_http.c (Arbeitskopie)
@@ -1481,12 +1481,18 @@
}
else if (rv != APR_SUCCESS) {
/* In this case, we are in real trouble because
- * our backend bailed on us, so abort our
- * connection to our user too.
+ * our backend bailed on us.
*/
ap_log_cerror(APLOG_MARK, APLOG_ERR, rv, c,
"proxy: error reading response");
- c->aborted = 1;
+ r->no_cache = 1;
+ e = ap_bucket_error_create(HTTP_BAD_GATEWAY, NULL,
+ c->pool, c->bucket_alloc);
+ APR_BRIGADE_INSERT_TAIL(bb, e);
+ e = apr_bucket_eos_create(c->bucket_alloc);
+ APR_BRIGADE_INSERT_TAIL(bb, e);
+ ap_pass_brigade(r->output_filters, bb);
+ backend->close = 1;
break;
}
/* next time try a non-blocking read */
Index: modules/cache/mod_disk_cache.c
===================================================================
--- modules/cache/mod_disk_cache.c (Revision 357328)
+++ 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.",
