Commit 8013d502 authored by Ruediger Pluem's avatar Ruediger Pluem
Browse files

Merge r484978,r484787,r484792 from trunk: 

* Isolate the unlock return vals

* Log when the lock fails.

* Failure to unlock is very nasty, so log it to help with troubleshooting.

Submitted by: jim
Reviewed by: jim, rpluem, wrowe


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@488822 13f79535-47bb-0310-9956-ffa450edef68
parent 41936f78

STATUS

+0 −10
Changes for STATUS: 0 added lines, 10 removed lines.
Original line number Diff line number Diff line
@@ -86,16 +86,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK: 
        http://mail-archives.apache.org/mod_mbox//httpd-dev/200609.mbox/%3c1404e5910609091218p84f4d2flc1000764b4966727@mail.gmail.com%3e

      +1: trawick, wrowe, jerenkrantz



   * mod_proxy: Log unlock errors (as we do with locking errors).

     Add additional clarification on where the error occurred.

     Trunk version of patch:

       http://svn.apache.org/viewvc?view=rev&revision=484792

       http://svn.apache.org/viewvc?view=rev&revision=484787

       http://svn.apache.org/viewvc?view=rev&revision=484978

     2.2.x version of patch:

       For r484787, see: http://people.apache.org/~jim/patches/lock.txt

       otherwise, trunk version works.

     +1: jim, rpluem, wrowe



PATCHES PROPOSED TO BACKPORT FROM TRUNK:

modules/proxy/mod_proxy_balancer.c

+28 −7
Changes for modules/proxy/mod_proxy_balancer.c: 28 added lines, 7 removed lines.
Original line number Diff line number Diff line
@@ -280,9 +280,13 @@ static proxy_worker *find_best_worker(proxy_balancer *balancer, 
                                      request_rec *r)

{

    proxy_worker *candidate = NULL;

    apr_status_t rv;



    if (PROXY_THREAD_LOCK(balancer) != APR_SUCCESS)

    if ((rv = PROXY_THREAD_LOCK(balancer)) != APR_SUCCESS) {

        ap_log_error(APLOG_MARK, APLOG_ERR, rv, r->server,

        "proxy: BALANCER: (%s). Lock failed for find_best_worker()", balancer->name);

        return NULL;

    }



    candidate = (*balancer->lbmethod->finder)(balancer, r);
@@ -294,7 +298,10 @@ static proxy_worker *find_best_worker(proxy_balancer *balancer, 
        return NULL;

*/



    PROXY_THREAD_UNLOCK(balancer);

    if ((rv = PROXY_THREAD_UNLOCK(balancer)) != APR_SUCCESS) {

        ap_log_error(APLOG_MARK, APLOG_ERR, rv, r->server,

        "proxy: BALANCER: (%s). Unlock failed for find_best_worker()", balancer->name);

    }



    if (candidate == NULL) {

        /* All the workers are in error state or disabled.
@@ -382,7 +389,8 @@ static int proxy_balancer_pre_request(proxy_worker **worker, 
     */

    if ((rv = PROXY_THREAD_LOCK(*balancer)) != APR_SUCCESS) {

        ap_log_error(APLOG_MARK, APLOG_ERR, rv, r->server,

                     "proxy: BALANCER: lock");

                     "proxy: BALANCER: (%s). Lock failed for pre_request",

                     (*balancer)->name);

        return DECLINED;

    }

    if (runtime) {
@@ -416,11 +424,19 @@ static int proxy_balancer_pre_request(proxy_worker **worker, 
        ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,

                     "proxy: BALANCER: (%s). All workers are in error state for route (%s)",

                     (*balancer)->name, route);

        PROXY_THREAD_UNLOCK(*balancer);

        if ((rv = PROXY_THREAD_UNLOCK(*balancer)) != APR_SUCCESS) {

            ap_log_error(APLOG_MARK, APLOG_ERR, rv, r->server,

                         "proxy: BALANCER: (%s). Unlock failed for pre_request",

                         (*balancer)->name);

        }

        return HTTP_SERVICE_UNAVAILABLE;

    }



    PROXY_THREAD_UNLOCK(*balancer);

    if ((rv = PROXY_THREAD_UNLOCK(*balancer)) != APR_SUCCESS) {

        ap_log_error(APLOG_MARK, APLOG_ERR, rv, r->server,

                     "proxy: BALANCER: (%s). Unlock failed for pre_request",

                     (*balancer)->name);

    }

    if (!*worker) {

        runtime = find_best_worker(*balancer, r);

        if (!runtime) {
@@ -485,7 +501,8 @@ static int proxy_balancer_post_request(proxy_worker *worker, 


    if ((rv = PROXY_THREAD_LOCK(balancer)) != APR_SUCCESS) {

        ap_log_error(APLOG_MARK, APLOG_ERR, rv, r->server,

            "proxy: BALANCER: lock");

            "proxy: BALANCER: (%s). Lock failed for post_request",

            balancer->name);

        return HTTP_INTERNAL_SERVER_ERROR;

    }

    /* TODO: calculate the bytes transferred
@@ -496,7 +513,11 @@ static int proxy_balancer_post_request(proxy_worker *worker, 
     * track on that.

     */



    PROXY_THREAD_UNLOCK(balancer);

    if ((rv = PROXY_THREAD_UNLOCK(balancer)) != APR_SUCCESS) {

        ap_log_error(APLOG_MARK, APLOG_ERR, rv, r->server,

            "proxy: BALANCER: (%s). Unlock failed for post_request",

            balancer->name);

    }

    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,

                 "proxy_balancer_post_request for (%s)", balancer->name);

modules/proxy/mod_proxy_ftp.c

+5 −1
Changes for modules/proxy/mod_proxy_ftp.c: 5 added lines, 1 removed line.
Original line number Diff line number Diff line
@@ -763,6 +763,7 @@ static int proxy_ftp_handler(request_rec *r, proxy_worker *worker, 
    apr_status_t rv;

    conn_rec *origin, *data = NULL;

    apr_status_t err = APR_SUCCESS;

    apr_status_t uerr = APR_SUCCESS;

    apr_bucket_brigade *bb = apr_brigade_create(p, c->bucket_alloc);

    char *buf, *connectname;

    apr_port_t connectport;
@@ -916,7 +917,10 @@ static int proxy_ftp_handler(request_rec *r, proxy_worker *worker, 
                                    address_pool);

    if (worker->is_address_reusable && !worker->cp->addr) {

        worker->cp->addr = connect_addr;

        PROXY_THREAD_UNLOCK(worker);

        if ((uerr = PROXY_THREAD_UNLOCK(worker)) != APR_SUCCESS) {

            ap_log_error(APLOG_MARK, APLOG_ERR, uerr, r->server,

                         "proxy: FTP: unlock");

        }

    }

    /*

     * get all the possible IP addresses for the destname and loop through

modules/proxy/proxy_util.c

+5 −1
Changes for modules/proxy/proxy_util.c: 5 added lines, 1 removed line.
Original line number Diff line number Diff line
@@ -1840,6 +1840,7 @@ ap_proxy_determine_connection(apr_pool_t *p, request_rec *r, 
{

    int server_port;

    apr_status_t err = APR_SUCCESS;

    apr_status_t uerr = APR_SUCCESS;



    /*

     * Break up the URL to determine the host to connect to
@@ -1922,7 +1923,10 @@ ap_proxy_determine_connection(apr_pool_t *p, request_rec *r, 
                                    conn->port, 0,

                                    worker->cp->pool);

        conn->addr = worker->cp->addr;

        PROXY_THREAD_UNLOCK(worker);

        if ((uerr = PROXY_THREAD_UNLOCK(worker)) != APR_SUCCESS) {

            ap_log_error(APLOG_MARK, APLOG_ERR, uerr, r->server,

                         "proxy: unlock");

        }

    }

    else

        conn->addr = worker->cp->addr;