I hacked up a patch that enables the proxy to keep connections persistent in the HTTPS case. Having no persistence here was a big performance penalty.
Basicly the persistence is created by keeping the conn_rec structure created for our backend connection (whether http or https) in the connection pool. This required to adjust scoreboard.c in a way that its functions can properly deal with a NULL scoreboard handle by ignoring the call or returning an error code. Thoughts, comments? Regards RĂ¼diger
Index: server/scoreboard.c =================================================================== --- server/scoreboard.c (revision 601660) +++ server/scoreboard.c (working copy) @@ -352,6 +352,9 @@ { worker_score *ws; + if (!sb) + return; + ws = &ap_scoreboard_image->servers[sb->child_num][sb->thread_num]; #ifdef HAVE_TIMES @@ -479,6 +482,9 @@ AP_DECLARE(int) ap_update_child_status(ap_sb_handle_t *sbh, int status, request_rec *r) { + if (!sbh) + return -1; + return ap_update_child_status_from_indexes(sbh->child_num, sbh->thread_num, status, r); } @@ -487,6 +493,9 @@ { worker_score *ws; + if (!sbh) + return; + if (sbh->child_num < 0) { return; } @@ -512,6 +521,9 @@ AP_DECLARE(worker_score *) ap_get_scoreboard_worker(ap_sb_handle_t *sbh) { + if (!sbh) + return NULL; + return ap_get_scoreboard_worker_from_indexes(sbh->child_num, sbh->thread_num); } Index: modules/proxy/proxy_util.c =================================================================== --- modules/proxy/proxy_util.c (revision 601660) +++ modules/proxy/proxy_util.c (working copy) @@ -1599,6 +1599,9 @@ /* determine if the connection need to be closed */ if (conn->close) { apr_pool_t *p = conn->pool; + if (conn->connection) { + apr_pool_cleanup_kill(conn->pool, conn, connection_cleanup); + } apr_pool_clear(conn->pool); memset(conn, 0, sizeof(proxy_conn_rec)); conn->pool = p; @@ -1619,6 +1622,54 @@ return APR_SUCCESS; } +static apr_status_t socket_cleanup(proxy_conn_rec *conn) +{ + if (conn->sock) { + apr_socket_close(conn->sock); + conn->sock = NULL; + } + if (conn->connection) { + apr_pool_cleanup_kill(conn->pool, conn, connection_cleanup); + conn->connection = NULL; + } + return APR_SUCCESS; +} + +PROXY_DECLARE(apr_status_t) ap_proxy_ssl_connection_cleanup(proxy_conn_rec *conn, + request_rec *r) +{ + apr_bucket_brigade *bb; + apr_status_t rv; + + /* + * If we have an existing SSL connection it might be possible that the + * server sent some SSL message we have not read so far (e.g. a SSL + * shutdown message if the server closed the keepalive connection while + * the connection was held unused in our pool). + * So ensure that if present (=> APR_NONBLOCK_READ) it is read and + * processed. We don't expect any data to be in the returned brigade. + */ + if (conn->sock && conn->connection) { + bb = apr_brigade_create(r->pool, r->connection->bucket_alloc); + rv = ap_get_brigade(conn->connection->input_filters, bb, + AP_MODE_READBYTES, APR_NONBLOCK_READ, + HUGE_STRING_LEN); + if ((rv != APR_SUCCESS) && !APR_STATUS_IS_EAGAIN(rv)) { + socket_cleanup(conn); + } + if (!APR_BRIGADE_EMPTY(bb)) { + apr_off_t len; + + rv = apr_brigade_length(bb, 0, &len); + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r, + "proxy: SSL cleanup brigade contained %" + APR_OFF_T_FMT " bytes of data.", len); + } + apr_brigade_destroy(bb); + } + return APR_SUCCESS; +} + /* reslist constructor */ static apr_status_t connection_constructor(void **resource, void *params, apr_pool_t *pool) @@ -1895,11 +1946,6 @@ ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, "proxy: %s: has released connection for (%s)", proxy_function, conn->worker->hostname); - /* If there is a connection kill it's cleanup */ - if (conn->connection) { - apr_pool_cleanup_kill(conn->connection->pool, conn, connection_cleanup); - conn->connection = NULL; - } connection_cleanup(conn); return OK; @@ -1972,14 +2018,7 @@ conn->hostname = apr_pstrdup(conn->pool, uri->hostname); conn->port = uri->port; } - if (conn->sock) { - apr_socket_close(conn->sock); - conn->sock = NULL; - } - if (conn->connection) { - apr_pool_cleanup_kill(conn->connection->pool, conn, connection_cleanup); - conn->connection = NULL; - } + socket_cleanup(conn); err = apr_sockaddr_info_get(&(conn->addr), conn->hostname, APR_UNSPEC, conn->port, 0, @@ -2131,8 +2170,7 @@ * relatively small compared to connection lifetime */ if (!(connected = is_socket_connected(conn->sock))) { - apr_socket_close(conn->sock); - conn->sock = NULL; + socket_cleanup(conn); ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, "proxy: %s: backend socket is disconnected.", proxy_function); @@ -2156,6 +2194,7 @@ backend_addr = backend_addr->next; continue; } + conn->connection = NULL; #if !defined(TPF) && !defined(BEOS) if (worker->recv_buffer_size > 0 && @@ -2245,13 +2284,19 @@ apr_sockaddr_t *backend_addr = conn->addr; int rc; apr_interval_time_t current_timeout; + apr_bucket_alloc_t *bucket_alloc; + if (conn->connection) { + return OK; + } + + bucket_alloc = apr_bucket_alloc_create(conn->pool); /* * The socket is now open, create a new backend server connection */ - conn->connection = ap_run_create_connection(c->pool, s, conn->sock, - c->id, c->sbh, - c->bucket_alloc); + conn->connection = ap_run_create_connection(conn->pool, s, conn->sock, + 0, NULL, + bucket_alloc); if (!conn->connection) { /* @@ -2263,15 +2308,14 @@ "new connection to %pI (%s)", proxy_function, backend_addr, conn->hostname); /* XXX: Will be closed when proxy_conn is closed */ - apr_socket_close(conn->sock); - conn->sock = NULL; + socket_cleanup(conn); return HTTP_INTERNAL_SERVER_ERROR; } /* * register the connection cleanup to client connection * so that the connection can be closed or reused */ - apr_pool_cleanup_register(c->pool, (void *)conn, + apr_pool_cleanup_register(conn->pool, (void *)conn, connection_cleanup, apr_pool_cleanup_null); Index: modules/proxy/mod_proxy_http.c =================================================================== --- modules/proxy/mod_proxy_http.c (revision 601660) +++ modules/proxy/mod_proxy_http.c (working copy) @@ -1824,15 +1824,11 @@ backend->is_ssl = is_ssl; - /* - * TODO: Currently we cannot handle persistent SSL backend connections, - * because we recreate backend->connection for each request and thus - * try to initialize an already existing SSL connection. This does - * not work. - */ - if (is_ssl) - backend->close = 1; + if (is_ssl) { + ap_proxy_ssl_connection_cleanup(backend, r); + } + /* Step One: Determine Who To Connect To */ if ((status = ap_proxy_determine_connection(p, r, conf, worker, backend, uri, &url, proxyname, Index: modules/proxy/mod_proxy.h =================================================================== --- modules/proxy/mod_proxy.h (revision 601660) +++ modules/proxy/mod_proxy.h (working copy) @@ -483,6 +483,8 @@ PROXY_DECLARE(void) ap_proxy_table_unmerge(apr_pool_t *p, apr_table_t *t, char *key); /* DEPRECATED (will be replaced with ap_proxy_connect_backend */ PROXY_DECLARE(int) ap_proxy_connect_to_backend(apr_socket_t **, const char *, apr_sockaddr_t *, const char *, proxy_server_conf *, server_rec *, apr_pool_t *); +PROXY_DECLARE(apr_status_t) ap_proxy_ssl_connection_cleanup(proxy_conn_rec *conn, + request_rec *r); PROXY_DECLARE(int) ap_proxy_ssl_enable(conn_rec *c); PROXY_DECLARE(int) ap_proxy_ssl_disable(conn_rec *c); PROXY_DECLARE(int) ap_proxy_conn_is_https(conn_rec *c); Index: include/ap_mmn.h =================================================================== --- include/ap_mmn.h (revision 601660) +++ include/ap_mmn.h (working copy) @@ -143,6 +143,7 @@ * 20071108.2 (2.3.0-dev) Add st and keep fields to struct util_ldap_connection_t * 20071108.3 (2.3.0-dev) Add API guarantee for adding connection filters * with non-NULL request_rec pointer (ap_add_*_filter*) + * 20071108.4 (2.3.0-dev) Add ap_proxy_ssl_connection_cleanup */ #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */ @@ -150,7 +151,7 @@ #ifndef MODULE_MAGIC_NUMBER_MAJOR #define MODULE_MAGIC_NUMBER_MAJOR 20071108 #endif -#define MODULE_MAGIC_NUMBER_MINOR 3 /* 0...n */ +#define MODULE_MAGIC_NUMBER_MINOR 4 /* 0...n */ /** * Determine if the server's current MODULE_MAGIC_NUMBER is at least a