From 3f795f76e87549183ee2985754857e23750d7e5f Mon Sep 17 00:00:00 2001 From: Olivier Houchard Date: Wed, 17 Apr 2019 22:51:06 +0200 Subject: [PATCH] MEDIUM: tasks: Merge task_delete() and task_free() into task_destroy(). task_delete() was never used without calling task_free() just after, and task_free() was only used on error pathes to destroy a just-created task, so merge them into task_destroy(), that will remove the task from the wait queue, and make sure the task is either destroyed immediately if it's not in the run queue, or destroyed when it's supposed to run. --- include/proto/applet.h | 3 +-- include/proto/task.h | 20 ++++++++------------ src/checks.c | 3 +-- src/dns.c | 3 +-- src/flt_spoe.c | 5 ++--- src/haproxy.c | 29 +++++++++++------------------ src/hlua.c | 15 +++++---------- src/mux_h1.c | 5 ++--- src/mux_h2.c | 5 ++--- src/mworker.c | 5 ++--- src/peers.c | 3 +-- src/session.c | 6 ++---- src/stream.c | 5 ++--- src/task.c | 12 ++++-------- 14 files changed, 44 insertions(+), 75 deletions(-) diff --git a/include/proto/applet.h b/include/proto/applet.h index 606f4b1bf1..91d7be25bf 100644 --- a/include/proto/applet.h +++ b/include/proto/applet.h @@ -85,8 +85,7 @@ static inline struct appctx *appctx_new(struct applet *applet, unsigned long thr */ static inline void __appctx_free(struct appctx *appctx) { - task_delete(appctx->t); - task_free(appctx->t); + task_destroy(appctx->t); if (!LIST_ISEMPTY(&appctx->buffer_wait.list)) { HA_SPIN_LOCK(BUF_WQ_LOCK, &buffer_wq_lock); LIST_DEL(&appctx->buffer_wait.list); diff --git a/include/proto/task.h b/include/proto/task.h index ab1245f825..badd130844 100644 --- a/include/proto/task.h +++ b/include/proto/task.h @@ -273,17 +273,6 @@ static inline void task_remove_from_tasklet_list(struct task *t) __task_remove_from_tasklet_list(t); } -/* - * Unlinks the task and adjusts run queue stats. - * A pointer to the task itself is returned. - */ -static inline struct task *task_delete(struct task *t) -{ - task_unlink_wq(t); - task_unlink_rq(t); - return t; -} - /* * Initialize a new task. The bare minimum is performed (queue pointers and * state). The task is returned. This function should not be used outside of @@ -350,8 +339,15 @@ static inline void __task_free(struct task *t) _HA_ATOMIC_SUB(&nb_tasks, 1); } -static inline void task_free(struct task *t) +static inline void task_destroy(struct task *t) { + task_unlink_wq(t); + /* We don't have to explicitely remove from the run queue. + * If we are in the runqueue, the test below will set t->process + * to NULL, and the task will be free'd when it'll be its turn + * to run. + */ + /* There's no need to protect t->state with a lock, as the task * has to run on the current thread. */ diff --git a/src/checks.c b/src/checks.c index 31004ddf89..76bd8e3df4 100644 --- a/src/checks.c +++ b/src/checks.c @@ -3301,8 +3301,7 @@ int init_email_alert(struct mailers *mls, struct proxy *p, char **err) struct check *check = &q->check; if (check->task) { - task_delete(check->task); - task_free(check->task); + task_destroy(check->task); check->task = NULL; } free_check(check); diff --git a/src/dns.c b/src/dns.c index 1d91e43819..274fa83b9a 100644 --- a/src/dns.c +++ b/src/dns.c @@ -1841,8 +1841,7 @@ static void dns_deinit(void) free(resolvers->id); free((char *)resolvers->conf.file); - task_delete(resolvers->t); - task_free(resolvers->t); + task_destroy(resolvers->t); LIST_DEL(&resolvers->list); free(resolvers); } diff --git a/src/flt_spoe.c b/src/flt_spoe.c index 1b517793ea..0f72de8cde 100644 --- a/src/flt_spoe.c +++ b/src/flt_spoe.c @@ -1276,8 +1276,7 @@ spoe_release_appctx(struct appctx *appctx) /* Destroy the task attached to this applet */ if (spoe_appctx->task) { - task_delete(spoe_appctx->task); - task_free(spoe_appctx->task); + task_destroy(spoe_appctx->task); } /* Notify all waiting streams */ @@ -2023,7 +2022,7 @@ spoe_create_appctx(struct spoe_config *conf) out_free_sess: session_free(sess); out_free_spoe: - task_free(SPOE_APPCTX(appctx)->task); + task_destroy(SPOE_APPCTX(appctx)->task); out_free_spoe_appctx: pool_free(pool_head_spoe_appctx, SPOE_APPCTX(appctx)); out_free_appctx: diff --git a/src/haproxy.c b/src/haproxy.c index 1b7b70c77e..3477ca54e4 100644 --- a/src/haproxy.c +++ b/src/haproxy.c @@ -2278,19 +2278,13 @@ void deinit(void) while (s) { s_next = s->next; - if (s->check.task) { - task_delete(s->check.task); - task_free(s->check.task); - } - if (s->agent.task) { - task_delete(s->agent.task); - task_free(s->agent.task); - } + if (s->check.task) + task_destroy(s->check.task); + if (s->agent.task) + task_destroy(s->agent.task); - if (s->warmup) { - task_delete(s->warmup); - task_free(s->warmup); - } + if (s->warmup) + task_destroy(s->warmup); free(s->id); free(s->cookie); @@ -2352,7 +2346,7 @@ void deinit(void) free_http_req_rules(&p->http_req_rules); free_http_res_rules(&p->http_res_rules); - task_free(p->task); + task_destroy(p->task); pool_destroy(p->req_cap_pool); pool_destroy(p->rsp_cap_pool); @@ -2398,8 +2392,8 @@ void deinit(void) free(global.node); global.node = NULL; free(global.desc); global.desc = NULL; free(oldpids); oldpids = NULL; - task_free(global_listener_queue_task); global_listener_queue_task = NULL; - task_free(idle_conn_task); + task_destroy(global_listener_queue_task); global_listener_queue_task = NULL; + task_destroy(idle_conn_task); idle_conn_task = NULL; list_for_each_entry_safe(log, logb, &global.logsrvs, list) { @@ -3080,10 +3074,9 @@ int main(int argc, char **argv) stop_proxy(curpeers->peers_fe); /* disable this peer section so that it kills itself */ signal_unregister_handler(curpeers->sighandler); - task_delete(curpeers->sync_task); - task_free(curpeers->sync_task); + task_destroy(curpeers->sync_task); curpeers->sync_task = NULL; - task_free(curpeers->peers_fe->task); + task_destroy(curpeers->peers_fe->task); curpeers->peers_fe->task = NULL; curpeers->peers_fe = NULL; } diff --git a/src/hlua.c b/src/hlua.c index 98c434ae24..ff5ffba553 100644 --- a/src/hlua.c +++ b/src/hlua.c @@ -6185,8 +6185,7 @@ static struct task *hlua_process_task(struct task *task, void *context, unsigned /* finished or yield */ case HLUA_E_OK: hlua_ctx_destroy(hlua); - task_delete(task); - task_free(task); + task_destroy(task); task = NULL; break; @@ -6199,8 +6198,7 @@ static struct task *hlua_process_task(struct task *task, void *context, unsigned case HLUA_E_ERRMSG: SEND_ERR(NULL, "Lua task: %s.\n", lua_tostring(hlua->T, -1)); hlua_ctx_destroy(hlua); - task_delete(task); - task_free(task); + task_destroy(task); task = NULL; break; @@ -6208,8 +6206,7 @@ static struct task *hlua_process_task(struct task *task, void *context, unsigned default: SEND_ERR(NULL, "Lua task: unknown error.\n"); hlua_ctx_destroy(hlua); - task_delete(task); - task_free(task); + task_destroy(task); task = NULL; break; } @@ -7052,8 +7049,7 @@ error: static void hlua_applet_tcp_release(struct appctx *ctx) { - task_delete(ctx->ctx.hlua_apptcp.task); - task_free(ctx->ctx.hlua_apptcp.task); + task_destroy(ctx->ctx.hlua_apptcp.task); ctx->ctx.hlua_apptcp.task = NULL; hlua_ctx_destroy(ctx->ctx.hlua_apptcp.hlua); ctx->ctx.hlua_apptcp.hlua = NULL; @@ -7520,8 +7516,7 @@ static void hlua_applet_http_fct(struct appctx *ctx) static void hlua_applet_http_release(struct appctx *ctx) { - task_delete(ctx->ctx.hlua_apphttp.task); - task_free(ctx->ctx.hlua_apphttp.task); + task_destroy(ctx->ctx.hlua_apphttp.task); ctx->ctx.hlua_apphttp.task = NULL; hlua_ctx_destroy(ctx->ctx.hlua_apphttp.hlua); ctx->ctx.hlua_apphttp.hlua = NULL; diff --git a/src/mux_h1.c b/src/mux_h1.c index b854fb2322..2fb2803bbb 100644 --- a/src/mux_h1.c +++ b/src/mux_h1.c @@ -432,7 +432,7 @@ static int h1_init(struct connection *conn, struct proxy *proxy, struct session fail: if (t) - task_free(t); + task_destroy(t); if (h1c->wait_event.task) tasklet_free(h1c->wait_event.task); pool_free(pool_head_h1c, h1c); @@ -1953,8 +1953,7 @@ static struct task *h1_timeout_task(struct task *t, void *context, unsigned shor if (!expired && h1c) return t; - task_delete(t); - task_free(t); + task_destroy(t); if (!h1c) { /* resources were already deleted */ diff --git a/src/mux_h2.c b/src/mux_h2.c index 0a500f6104..5d084728c0 100644 --- a/src/mux_h2.c +++ b/src/mux_h2.c @@ -576,7 +576,7 @@ static int h2_init(struct connection *conn, struct proxy *prx, struct session *s hpack_dht_free(h2c->ddht); fail: if (t) - task_free(t); + task_destroy(t); if (h2c->wait_event.task) tasklet_free(h2c->wait_event.task); pool_free(pool_head_h2c, h2c); @@ -2925,8 +2925,7 @@ static struct task *h2_timeout_task(struct task *t, void *context, unsigned shor if (!expired && h2c) return t; - task_delete(t); - task_free(t); + task_destroy(t); if (!h2c) { /* resources were already deleted */ diff --git a/src/mworker.c b/src/mworker.c index 6b591d4eb3..abc67bd8b8 100644 --- a/src/mworker.c +++ b/src/mworker.c @@ -379,10 +379,9 @@ void mworker_cleanlisteners() stop_proxy(curpeers->peers_fe); /* disable this peer section so that it kills itself */ signal_unregister_handler(curpeers->sighandler); - task_delete(curpeers->sync_task); - task_free(curpeers->sync_task); + task_destroy(curpeers->sync_task); curpeers->sync_task = NULL; - task_free(curpeers->peers_fe->task); + task_destroy(curpeers->peers_fe->task); curpeers->peers_fe->task = NULL; curpeers->peers_fe = NULL; } diff --git a/src/peers.c b/src/peers.c index ee597dcaa7..3f59b10ba7 100644 --- a/src/peers.c +++ b/src/peers.c @@ -2448,8 +2448,7 @@ static struct task *process_peer_sync(struct task * task, void *context, unsigne if (!peers->peers_fe) { /* this one was never started, kill it */ signal_unregister_handler(peers->sighandler); - task_delete(peers->sync_task); - task_free(peers->sync_task); + task_destroy(peers->sync_task); peers->sync_task = NULL; return NULL; } diff --git a/src/session.c b/src/session.c index 9b0db46b23..875fae89b7 100644 --- a/src/session.c +++ b/src/session.c @@ -396,8 +396,7 @@ static void session_kill_embryonic(struct session *sess, unsigned short state) conn_free(conn); sess->origin = NULL; - task_delete(task); - task_free(task); + task_destroy(task); session_free(sess); } @@ -450,8 +449,7 @@ static int conn_complete_session(struct connection *conn) /* the embryonic session's task is not needed anymore */ if (sess->task) { - task_delete(sess->task); - task_free(sess->task); + task_destroy(sess->task); sess->task = NULL; } diff --git a/src/stream.c b/src/stream.c index 551f8e2ccf..3762f1bbdb 100644 --- a/src/stream.c +++ b/src/stream.c @@ -333,7 +333,7 @@ struct stream *stream_new(struct session *sess, enum obj_type *origin) /* Error unrolling */ out_fail_accept: flt_stream_release(s, 0); - task_free(t); + task_destroy(t); tasklet_free(s->si[1].wait_event.task); LIST_DEL(&s->list); out_fail_alloc_si1: @@ -2616,8 +2616,7 @@ redo: /* the task MUST not be in the run queue anymore */ stream_free(s); - task_delete(t); - task_free(t); + task_destroy(t); return NULL; } diff --git a/src/task.c b/src/task.c index 52686cfb9d..f38807c7a1 100644 --- a/src/task.c +++ b/src/task.c @@ -432,16 +432,14 @@ void mworker_cleantasks() while (tmp_rq) { t = eb32sc_entry(tmp_rq, struct task, rq); tmp_rq = eb32sc_next(tmp_rq, MAX_THREADS_MASK); - task_delete(t); - task_free(t); + task_destroy(t); } /* cleanup the timers queue */ tmp_wq = eb32_first(&timers); while (tmp_wq) { t = eb32_entry(tmp_wq, struct task, wq); tmp_wq = eb32_next(tmp_wq); - task_delete(t); - task_free(t); + task_destroy(t); } #endif /* clean the per thread run queue */ @@ -450,16 +448,14 @@ void mworker_cleantasks() while (tmp_rq) { t = eb32sc_entry(tmp_rq, struct task, rq); tmp_rq = eb32sc_next(tmp_rq, MAX_THREADS_MASK); - task_delete(t); - task_free(t); + task_destroy(t); } /* cleanup the per thread timers queue */ tmp_wq = eb32_first(&task_per_thread[i].timers); while (tmp_wq) { t = eb32_entry(tmp_wq, struct task, wq); tmp_wq = eb32_next(tmp_wq); - task_delete(t); - task_free(t); + task_destroy(t); } } } -- 2.39.5