Currently I work on PR 38602
(http://issues.apache.org/bugzilla/show_bug.cgi?id=38602).
First of all the reporter is correct that we do not sent the
Connection: Keep-Alive
header on our HTTP/1.1 keep-alive connections to the backend.
But this is only the small part of the problem since 8.1.2 of the RFC says:
"A significant difference between HTTP/1.1 and earlier versions of
HTTP is that persistent connections are the default behavior of any
HTTP connection. That is, unless otherwise indicated, the client
SHOULD assume that the server will maintain a persistent connection,"
The real problem is that we actually *close* our connection to the backend
after each request (see line 1512 of mod_proxy_http.c) and if it would have
survived we would *close* it on the reusal of this connection:
lines 1852 - 1869 of proxy_util.c:
if (r->proxyreq == PROXYREQ_PROXY || r->proxyreq == PROXYREQ_REVERSE ||
!worker->is_address_reusable) {
/*
* TODO: Check if the connection can be reused
*/
if (conn->connection) {
if (conn->sock) {
apr_socket_close(conn->sock);
conn->sock = NULL;
}
apr_pool_cleanup_kill(conn->connection->pool, conn,
connection_cleanup);
conn->connection = NULL;
}
err = apr_sockaddr_info_get(&(conn->addr),
conn->hostname, APR_UNSPEC,
conn->port, 0,
conn->pool);
}
I tried to fix this behaviour and the attached patch works for me, but I have
the slight
feeling that I stired up a hornet's nest :-). So a review and comments would be
very
much appreciated to ensure that this does not break other things.
Regards
Rüdiger
Index: modules/proxy/proxy_util.c
===================================================================
--- modules/proxy/proxy_util.c (Revision 377059)
+++ modules/proxy/proxy_util.c (Arbeitskopie)
@@ -1844,16 +1844,6 @@
conn->hostname = apr_pstrdup(conn->pool, uri->hostname);
conn->port = uri->port;
}
- }
- /*
- * TODO: add address cache for generic forward proxies.
- * At least level 0 -> compare with previous hostname:port
- */
- if (r->proxyreq == PROXYREQ_PROXY || r->proxyreq == PROXYREQ_REVERSE ||
- !worker->is_address_reusable) {
- /*
- * TODO: Check if the connection can be reused
- */
if (conn->connection) {
if (conn->sock) {
apr_socket_close(conn->sock);
Index: modules/proxy/mod_proxy_http.c
===================================================================
--- modules/proxy/mod_proxy_http.c (Revision 377059)
+++ modules/proxy/mod_proxy_http.c (Arbeitskopie)
@@ -975,9 +975,18 @@
/* Yes I hate gotos. This is the subrequest shortcut */
skip_body:
- /* Handle Connection: header */
- if (!force10 && p_conn->close) {
- buf = apr_pstrdup(p, "Connection: close" CRLF);
+ /*
+ * Handle Connection: header if we do HTTP/1.1 request:
+ * If we plan to close the backend connection sent Connection: close
+ * otherwise sent Connection: Keep-Alive.
+ */
+ if (!force10) {
+ if (p_conn->close) {
+ buf = apr_pstrdup(p, "Connection: close" CRLF);
+ }
+ else {
+ buf = apr_pstrdup(p, "Connection: Keep-Alive" CRLF);
+ }
ap_xlate_proto_to_ascii(buf, strlen(buf));
e = apr_bucket_pool_create(buf, strlen(buf), p, c->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(header_brigade, e);
@@ -1504,12 +1513,6 @@
/* found the last brigade? */
if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(bb))) {
- /* if this is the last brigade, cleanup the
- * backend connection first to prevent the
- * backend server from hanging around waiting
- * for a slow client to eat these bytes
- */
- backend->close = 1;
/* signal that we must leave */
finish = TRUE;
}
@@ -1578,18 +1581,7 @@
apr_status_t ap_proxy_http_cleanup(const char *scheme, request_rec *r,
proxy_conn_rec *backend)
{
- /* If there are no KeepAlives, or if the connection has been signalled
- * to close, close the socket and clean up
- */
-
- /* if the connection is < HTTP/1.1, or Connection: close,
- * we close the socket, otherwise we leave it open for KeepAlive support
- */
- if (backend->close || (r->proto_num < HTTP_VERSION(1,1))) {
- backend->close_on_recycle = 1;
- ap_set_module_config(r->connection->conn_config, &proxy_http_module,
NULL);
- ap_proxy_release_connection(scheme, backend, r->server);
- }
+ ap_proxy_release_connection(scheme, backend, r->server);
return OK;
}
@@ -1667,26 +1659,15 @@
"proxy: HTTP: serving URL %s", url);
- /* only use stored info for top-level pages. Sub requests don't share
- * in keepalives
- */
- if (!r->main) {
- backend = (proxy_conn_rec *) ap_get_module_config(c->conn_config,
- &proxy_http_module);
- }
/* create space for state information */
if (!backend) {
if ((status = ap_proxy_acquire_connection(proxy_function, &backend,
worker, r->server)) != OK)
goto cleanup;
- if (!r->main) {
- ap_set_module_config(c->conn_config, &proxy_http_module, backend);
- }
}
backend->is_ssl = is_ssl;
- backend->close_on_recycle = 1;
/* Step One: Determine Who To Connect To */
if ((status = ap_proxy_determine_connection(p, r, conf, worker, backend,
@@ -1726,10 +1707,8 @@
cleanup:
if (backend) {
- if (status != OK) {
+ if (status != OK)
backend->close = 1;
- backend->close_on_recycle = 1;
- }
ap_proxy_http_cleanup(proxy_function, r, backend);
}
return status;
