]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MEDIUM: threads/server: Use the server lock to protect health check and cli concurrency
authorEmeric Brun <ebrun@haproxy.com>
Mon, 23 Oct 2017 12:39:51 +0000 (14:39 +0200)
committerWilly Tarreau <w@1wt.eu>
Tue, 31 Oct 2017 12:58:33 +0000 (13:58 +0100)
include/proto/checks.h
src/checks.c
src/hlua_fcn.c
src/server.c
src/stats.c

index 98dca25c7705554ce26a21dd75b42287d08f221f..b0b8c7d19bb7fe375985d9d971f6ccfa0d0d362e 100644 (file)
@@ -37,11 +37,15 @@ extern struct data_cb check_conn_cb;
  */
 static inline void health_adjust(struct server *s, short status)
 {
+       SPIN_LOCK(SERVER_LOCK, &s->lock);
        /* return now if observing nor health check is not enabled */
-       if (!s->observe || !s->check.task)
+       if (!s->observe || !s->check.task) {
+               SPIN_UNLOCK(SERVER_LOCK, &s->lock);
                return;
+       }
 
-       return __health_adjust(s, status);
+       __health_adjust(s, status);
+       SPIN_UNLOCK(SERVER_LOCK, &s->lock);
 }
 
 const char *init_check(struct check *check, int type);
index d07e508fcf0b1b4aa5e2a1727115fdb474268c96..8411fefae3b301ae47e52c9ed9632a8597936f6f 100644 (file)
@@ -650,6 +650,7 @@ static void chk_report_conn_err(struct check *check, int errno_bck, int expired)
                }
        }
 
+       SPIN_LOCK(SERVER_LOCK, &check->server->lock);
        if (check->state & CHK_ST_PORT_MISS) {
                /* NOTE: this is reported after <fall> tries */
                chunk_printf(chk, "No port available for the TCP connection");
@@ -692,6 +693,7 @@ static void chk_report_conn_err(struct check *check, int errno_bck, int expired)
                else    /* HTTP, SMTP, ... */
                        set_server_check_status(check, HCHK_STATUS_L7TOUT, err_msg);
        }
+       SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
 
        return;
 }
@@ -784,15 +786,21 @@ static void event_srv_chk_r(struct connection *conn)
        int done;
        unsigned short msglen;
 
+       SPIN_LOCK(SERVER_LOCK, &check->server->lock);
+
        if (unlikely(check->result == CHK_RES_FAILED))
                goto out_wakeup;
 
-       if (conn->flags & CO_FL_HANDSHAKE)
+       if (conn->flags & CO_FL_HANDSHAKE) {
+               SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
                return;
+       }
 
        /* wake() will take care of calling tcpcheck_main() */
-       if (check->type == PR_O2_TCPCHK_CHK)
+       if (check->type == PR_O2_TCPCHK_CHK) {
+               SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
                return;
+       }
 
        /* Warning! Linux returns EAGAIN on SO_ERROR if data are still available
         * but the connection was closed on the remote end. Fortunately, recv still
@@ -896,7 +904,6 @@ static void event_srv_chk_r(struct connection *conn)
                    !isdigit((unsigned char) *(check->bi->data + 2))) {
                        cut_crlf(check->bi->data);
                        set_server_check_status(check, HCHK_STATUS_L7RSP, check->bi->data);
-
                        goto out_wakeup;
                }
 
@@ -1163,6 +1170,7 @@ static void event_srv_chk_r(struct connection *conn)
                                else {
                                        if (!done)
                                                goto wait_more_data;
+
                                        /* it seems we have a OK packet but without a valid length,
                                         * it must be a protocol error
                                         */
@@ -1224,6 +1232,7 @@ static void event_srv_chk_r(struct connection *conn)
                        else {
                                if (!done)
                                        goto wait_more_data;
+
                                /* it seems we have a Handshake Initialization packet but without a valid length,
                                 * it must be a protocol error
                                 */
@@ -1260,7 +1269,6 @@ static void event_srv_chk_r(struct connection *conn)
                        if ((msglen > 2) ||
                            (memcmp(check->bi->data + 2 + msglen, "\x02\x01\x01\x61", 4) != 0)) {
                                set_server_check_status(check, HCHK_STATUS_L7RSP, "Not LDAPv3 protocol");
-
                                goto out_wakeup;
                        }
 
@@ -1273,7 +1281,6 @@ static void event_srv_chk_r(struct connection *conn)
                        if ((msglen > 4) ||
                            (memcmp(check->bi->data + 7 + msglen, "\x0a\x01", 2) != 0)) {
                                set_server_check_status(check, HCHK_STATUS_L7RSP, "Not LDAPv3 protocol");
-
                                goto out_wakeup;
                        }
 
@@ -1314,6 +1321,7 @@ static void event_srv_chk_r(struct connection *conn)
                break;
        } /* switch */
 
+       SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
  out_wakeup:
        /* collect possible new errors */
        if (conn->flags & CO_FL_ERROR)
index a4adb21781be05cceec706c0f746e34ebe459680..60becb02f1489e79200f5483b51f247c7be60ad9 100644 (file)
@@ -626,7 +626,9 @@ int hlua_server_shut_sess(lua_State *L)
        struct server *srv;
 
        srv = hlua_check_server(L, 1);
+       SPIN_LOCK(SERVER_LOCK, &srv->lock);
        srv_shutdown_streams(srv, SF_ERR_KILLED);
+       SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
        return 0;
 }
 
@@ -635,7 +637,9 @@ int hlua_server_set_drain(lua_State *L)
        struct server *srv;
 
        srv = hlua_check_server(L, 1);
+       SPIN_LOCK(SERVER_LOCK, &srv->lock);
        srv_adm_set_drain(srv);
+       SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
        return 0;
 }
 
@@ -644,7 +648,9 @@ int hlua_server_set_maint(lua_State *L)
        struct server *srv;
 
        srv = hlua_check_server(L, 1);
+       SPIN_LOCK(SERVER_LOCK, &srv->lock);
        srv_adm_set_maint(srv);
+       SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
        return 0;
 }
 
@@ -653,7 +659,9 @@ int hlua_server_set_ready(lua_State *L)
        struct server *srv;
 
        srv = hlua_check_server(L, 1);
+       SPIN_LOCK(SERVER_LOCK, &srv->lock);
        srv_adm_set_ready(srv);
+       SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
        return 0;
 }
 
@@ -662,9 +670,11 @@ int hlua_server_check_enable(lua_State *L)
        struct server *sv;
 
        sv = hlua_check_server(L, 1);
+       SPIN_LOCK(SERVER_LOCK, &sv->lock);
        if (sv->check.state & CHK_ST_CONFIGURED) {
                sv->check.state |= CHK_ST_ENABLED;
        }
+       SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
        return 0;
 }
 
@@ -673,9 +683,11 @@ int hlua_server_check_disable(lua_State *L)
        struct server *sv;
 
        sv = hlua_check_server(L, 1);
+       SPIN_LOCK(SERVER_LOCK, &sv->lock);
        if (sv->check.state & CHK_ST_CONFIGURED) {
                sv->check.state &= ~CHK_ST_ENABLED;
        }
+       SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
        return 0;
 }
 
@@ -685,8 +697,10 @@ int hlua_server_check_force_up(lua_State *L)
 
        sv = hlua_check_server(L, 1);
        if (!(sv->track)) {
+               SPIN_LOCK(SERVER_LOCK, &sv->lock);
                sv->check.health = sv->check.rise + sv->check.fall - 1;
                srv_set_running(sv, "changed from Lua script", NULL);
+               SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
        }
        return 0;
 }
@@ -697,8 +711,10 @@ int hlua_server_check_force_nolb(lua_State *L)
 
        sv = hlua_check_server(L, 1);
        if (!(sv->track)) {
+               SPIN_LOCK(SERVER_LOCK, &sv->lock);
                sv->check.health = sv->check.rise + sv->check.fall - 1;
                srv_set_stopping(sv, "changed from Lua script", NULL);
+               SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
        }
        return 0;
 }
@@ -708,10 +724,12 @@ int hlua_server_check_force_down(lua_State *L)
        struct server *sv;
 
        sv = hlua_check_server(L, 1);
+       SPIN_LOCK(SERVER_LOCK, &sv->lock);
        if (!(sv->track)) {
                sv->check.health = 0;
                srv_set_stopped(sv, "changed from Lua script", NULL);
        }
+       SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
        return 0;
 }
 
@@ -720,9 +738,11 @@ int hlua_server_agent_enable(lua_State *L)
        struct server *sv;
 
        sv = hlua_check_server(L, 1);
+       SPIN_LOCK(SERVER_LOCK, &sv->lock);
        if (sv->agent.state & CHK_ST_CONFIGURED) {
                sv->agent.state |= CHK_ST_ENABLED;
        }
+       SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
        return 0;
 }
 
@@ -731,9 +751,11 @@ int hlua_server_agent_disable(lua_State *L)
        struct server *sv;
 
        sv = hlua_check_server(L, 1);
+       SPIN_LOCK(SERVER_LOCK, &sv->lock);
        if (sv->agent.state & CHK_ST_CONFIGURED) {
                sv->agent.state &= ~CHK_ST_ENABLED;
        }
+       SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
        return 0;
 }
 
@@ -742,10 +764,12 @@ int hlua_server_agent_force_up(lua_State *L)
        struct server *sv;
 
        sv = hlua_check_server(L, 1);
+       SPIN_LOCK(SERVER_LOCK, &sv->lock);
        if (sv->agent.state & CHK_ST_ENABLED) {
                sv->agent.health = sv->agent.rise + sv->agent.fall - 1;
                srv_set_running(sv, "changed from Lua script", NULL);
        }
+       SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
        return 0;
 }
 
@@ -754,10 +778,12 @@ int hlua_server_agent_force_down(lua_State *L)
        struct server *sv;
 
        sv = hlua_check_server(L, 1);
+       SPIN_LOCK(SERVER_LOCK, &sv->lock);
        if (sv->agent.state & CHK_ST_ENABLED) {
                sv->agent.health = 0;
                srv_set_stopped(sv, "changed from Lua script", NULL);
        }
+       SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
        return 0;
 }
 
index f0b912f66dd5a130046adae06cf74f682d6d4420..2d0e3b4f90ad3b66ac4f2131c12bffeff3ac2902 100644 (file)
@@ -880,8 +880,11 @@ void srv_set_stopped(struct server *s, const char *reason, struct check *check)
        }
 
        srv_register_update(s);
-       for (srv = s->trackers; srv; srv = srv->tracknext)
+       for (srv = s->trackers; srv; srv = srv->tracknext) {
+               SPIN_LOCK(SERVER_LOCK, &srv->lock);
                srv_set_stopped(srv, NULL, NULL);
+               SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
+       }
 }
 
 /* Marks server <s> up regardless of its checks' statuses and provided it isn't
@@ -919,8 +922,11 @@ void srv_set_running(struct server *s, const char *reason, struct check *check)
                s->next_state = SRV_ST_RUNNING;
 
        srv_register_update(s);
-       for (srv = s->trackers; srv; srv = srv->tracknext)
+       for (srv = s->trackers; srv; srv = srv->tracknext) {
+               SPIN_LOCK(SERVER_LOCK, &srv->lock);
                srv_set_running(srv, NULL, NULL);
+               SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
+       }
 }
 
 /* Marks server <s> stopping regardless of its checks' statuses and provided it
@@ -957,8 +963,11 @@ void srv_set_stopping(struct server *s, const char *reason, struct check *check)
        }
 
        srv_register_update(s);
-       for (srv = s->trackers; srv; srv = srv->tracknext)
+       for (srv = s->trackers; srv; srv = srv->tracknext) {
+               SPIN_LOCK(SERVER_LOCK, &srv->lock);
                srv_set_stopping(srv, NULL, NULL);
+               SPIN_LOCK(SERVER_LOCK, &srv->lock);
+       }
 }
 
 /* Enables admin flag <mode> (among SRV_ADMF_*) on server <s>. This is used to
@@ -997,8 +1006,11 @@ void srv_set_admin_flag(struct server *s, enum srv_admin mode, const char *cause
        else if (mode & SRV_ADMF_DRAIN)
                mode = SRV_ADMF_IDRAIN;
 
-       for (srv = s->trackers; srv; srv = srv->tracknext)
+       for (srv = s->trackers; srv; srv = srv->tracknext) {
+               SPIN_LOCK(SERVER_LOCK, &srv->lock);
                srv_set_admin_flag(srv, mode, cause);
+               SPIN_LOCK(SERVER_LOCK, &srv->lock);
+       }
 }
 
 /* Disables admin flag <mode> (among SRV_ADMF_*) on server <s>. This is used to
@@ -1032,8 +1044,11 @@ void srv_clr_admin_flag(struct server *s, enum srv_admin mode)
        else if (mode & SRV_ADMF_DRAIN)
                mode = SRV_ADMF_IDRAIN;
 
-       for (srv = s->trackers; srv; srv = srv->tracknext)
+       for (srv = s->trackers; srv; srv = srv->tracknext) {
+               SPIN_LOCK(SERVER_LOCK, &srv->lock);
                srv_clr_admin_flag(srv, mode);
+               SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
+       }
 }
 
 /* principle: propagate maint and drain to tracking servers. This is useful
@@ -1047,11 +1062,13 @@ static void srv_propagate_admin_state(struct server *srv)
                return;
 
        for (srv2 = srv->trackers; srv2; srv2 = srv2->tracknext) {
+               SPIN_LOCK(SERVER_LOCK, &srv2->lock);
                if (srv->next_admin & (SRV_ADMF_MAINT | SRV_ADMF_CMAINT))
                        srv_set_admin_flag(srv2, SRV_ADMF_IMAINT, NULL);
 
                if (srv->next_admin & SRV_ADMF_DRAIN)
                        srv_set_admin_flag(srv2, SRV_ADMF_IDRAIN, NULL);
+               SPIN_UNLOCK(SERVER_LOCK, &srv2->lock);
        }
 }
 
@@ -2772,6 +2789,7 @@ static void srv_update_state(struct server *srv, int version, char **params)
                        if (msg->len)
                                goto out;
 
+                       SPIN_LOCK(SERVER_LOCK, &srv->lock);
                        /* recover operational state and apply it to this server
                         * and all servers tracking this one */
                        switch (srv_op_state) {
@@ -2901,6 +2919,7 @@ static void srv_update_state(struct server *srv, int version, char **params)
 
                        if (port_str)
                                srv->svc_port = port;
+                       SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
 
                        break;
                default:
@@ -4013,6 +4032,8 @@ static int cli_parse_set_server(char **args, struct appctx *appctx, void *privat
        if (!sv)
                return 1;
 
+       SPIN_LOCK(SERVER_LOCK, &sv->lock);
+
        if (strcmp(args[3], "weight") == 0) {
                warning = server_parse_weight_change_request(sv, args[4]);
                if (warning) {
@@ -4126,6 +4147,7 @@ static int cli_parse_set_server(char **args, struct appctx *appctx, void *privat
                        appctx->ctx.cli.severity = LOG_ERR;
                        appctx->ctx.cli.msg = "can't unset 'port' since MAPPORTS is in use.\n";
                        appctx->st0 = CLI_ST_PRINT;
+                       SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
                        return 1;
                }
                sv->check.port = i;
@@ -4140,6 +4162,7 @@ static int cli_parse_set_server(char **args, struct appctx *appctx, void *privat
                        appctx->ctx.cli.severity = LOG_ERR;
                        appctx->ctx.cli.msg = "set server <b>/<s> addr requires an address and optionally a port.\n";
                        appctx->st0 = CLI_ST_PRINT;
+                       SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
                        return 1;
                }
                else {
@@ -4174,6 +4197,7 @@ static int cli_parse_set_server(char **args, struct appctx *appctx, void *privat
                appctx->ctx.cli.msg = "'set server <srv>' only supports 'agent', 'health', 'state', 'weight', 'addr', 'fqdn' and 'check-port'.\n";
                appctx->st0 = CLI_ST_PRINT;
        }
+       SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
        return 1;
 }
 
index e026f36a55b11dbc62504f82622e59a448327e43..f688319339efbeec3b1583007babba31ea5dcecb 100644 (file)
@@ -2764,6 +2764,7 @@ static int stats_process_http_post(struct stream_interface *si)
                                        reprocess = 1;
                                }
                                else if ((sv = findserver(px, value)) != NULL) {
+                                       SPIN_LOCK(SERVER_LOCK, &sv->lock);
                                        switch (action) {
                                        case ST_ADM_ACTION_DISABLE:
                                                if (!(sv->cur_admin & SRV_ADMF_FMAINT)) {
@@ -2880,17 +2881,16 @@ static int stats_process_http_post(struct stream_interface *si)
                                                if (px->state != PR_STSTOPPED) {
                                                        struct stream *sess, *sess_bck;
 
-                                                       SPIN_LOCK(SERVER_LOCK, &sv->lock);
                                                        list_for_each_entry_safe(sess, sess_bck, &sv->actconns, by_srv)
                                                                if (sess->srv_conn == sv)
                                                                        stream_shutdown(sess, SF_ERR_KILLED);
-                                                       SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
 
                                                        altered_servers++;
                                                        total_servers++;
                                                }
                                                break;
                                        }
+                                       SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
                                } else {
                                        /* the server name is unknown or ambiguous (duplicate names) */
                                        total_servers++;