From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 5D93D309E8 for ; Wed, 5 Jun 2019 17:54:00 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id r2CPj7AQpuwv for ; Wed, 5 Jun 2019 17:54:00 -0400 (EDT) Received: from smtp53.i.mail.ru (smtp53.i.mail.ru [94.100.177.113]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id ECE7B2F64A for ; Wed, 5 Jun 2019 17:53:59 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH 3/5] swim: allow to hang triggers on member updates References: <20190605071159.GD28736@atlas> From: Vladislav Shpilevoy Message-ID: <410cb69a-a2c4-9fb0-8e85-e6f610c25522@tarantool.org> Date: Wed, 5 Jun 2019 23:53:56 +0200 MIME-Version: 1.0 In-Reply-To: <20190605071159.GD28736@atlas> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: Konstantin Osipov Cc: tarantool-patches@freelists.org On 05/06/2019 10:11, Konstantin Osipov wrote: > * Vladislav Shpilevoy [19/06/03 14:33]: > > I like the design: elegant and lean. Some comments below. > Thanks. >> This commit allows to hang triggers on member updates. > > "hang triggers" sounds ominous. I think you simply > meant "set triggers". See also patch subject, please change as > well. > Yes, 'set' is the right word. Updated in all places. >> struct rlist in_dissemination_queue; >> + /** >> + * Each time a member is updated, or created, or dropped, >> + * it is added to an update queue. Members from this queue >> + * are dispatched into user defined triggers. >> + */ >> + struct stailq_entry in_update_queue; > > I think the name is a bit confusing. After all, the dissemination > queue is also an update queue. in_local_queue? in_trigger_queue? > in_event_queue? in_pending_member_update_queue? > in_member_update? in_member_update_queue? Indeed, 'update' is not the best name. I guess, 'event_queue' is better - it is consistent with 'events' field in struct swim_member. Besides, when a member is dropped, technically we can't call it 'update'. But then we should rename everything, including 'swim_trigger_list_on_member_update', and Lua part 'swim:on_member_update'. They will be 'swim_trigger_list_on_member_event', 'swim:on_member_event' etc. ======================================================================= diff --git a/src/lib/swim/swim.c b/src/lib/swim/swim.c index 8507eea14..d48e43761 100644 --- a/src/lib/swim/swim.c +++ b/src/lib/swim/swim.c @@ -337,10 +337,10 @@ struct swim_member { struct rlist in_dissemination_queue; /** * Each time a member is updated, or created, or dropped, - * it is added to an update queue. Members from this queue + * it is added to an event queue. Members from this queue * are dispatched into user defined triggers. */ - struct stailq_entry in_update_queue; + struct stailq_entry in_event_queue; /** * Mask of events happened with this member since a * previous trigger invocation. Once the events are @@ -474,12 +474,12 @@ struct swim { * the events to triggers. Dropped members are also kept * here until they are handled by a trigger. */ - struct stailq update_queue; + struct stailq event_queue; /** * List of triggers to call on each new, dropped, and * updated member. */ - struct rlist on_member_update; + struct rlist on_member_event; /** * Members to which a message should be sent next during * this round. @@ -498,10 +498,10 @@ struct swim { */ struct swim_member **shuffled; /** - * Worker fiber to serve member update triggers. This task + * Worker fiber to serve member event triggers. This task * is being done in a separate fiber, because user * triggers can yield and libev callbacks, processing - * member updates, are not allowed to yield. + * member events, are not allowed to yield. */ struct fiber *worker; /** @@ -587,10 +587,10 @@ swim_on_member_update(struct swim *swim, struct swim_member *member, member->unacknowledged_pings = 0; swim_register_event(swim, member); /* - * Member update should be delivered to triggers only - * if there is at least one trigger. + * Member event should be delivered to triggers only if + * there is at least one trigger. */ - if (! rlist_empty(&swim->on_member_update)) { + if (! rlist_empty(&swim->on_member_event)) { /* * Member is referenced and added to a queue only * once. That moment can be detected when a first @@ -598,8 +598,8 @@ swim_on_member_update(struct swim *swim, struct swim_member *member, */ if (member->events == 0 && events != 0) { swim_member_ref(member); - stailq_add_tail_entry(&swim->update_queue, member, - in_update_queue); + stailq_add_tail_entry(&swim->event_queue, member, + in_event_queue); fiber_wakeup(swim->worker); } member->events |= events; @@ -607,15 +607,15 @@ swim_on_member_update(struct swim *swim, struct swim_member *member, } struct rlist * -swim_trigger_list_on_member_update(struct swim *swim) +swim_trigger_list_on_member_event(struct swim *swim) { - return &swim->on_member_update; + return &swim->on_member_event; } bool swim_has_pending_events(struct swim *swim) { - return ! stailq_empty(&swim->update_queue); + return ! stailq_empty(&swim->event_queue); } /** @@ -1744,7 +1744,7 @@ swim_do_delete(struct swim *swim); /** * Worker fiber. At this moment its only task is dispatching - * member updates to user defined triggers. Generally, because + * member events to user defined triggers. Generally, because * SWIM is fully IO driven, that fiber should be used only for * yielding tasks not related to SWIM core logic. For all the * other tasks libev callbacks are ok. Unfortunately, yields are @@ -1755,22 +1755,22 @@ static int swim_worker_f(va_list va) { struct swim *s = va_arg(va, struct swim *); - struct swim_on_member_update_ctx ctx; + struct swim_on_member_event_ctx ctx; while (! fiber_is_cancelled()) { - while (stailq_empty(&s->update_queue)) { + while (stailq_empty(&s->event_queue)) { fiber_yield(); if (fiber_is_cancelled()) goto end; } /* * Can't be empty. SWIM deletes members from - * update queue only on SWIM deletion, but then + * event queue only on SWIM deletion, but then * the fiber would be stopped already. */ - assert(! stailq_empty(&s->update_queue)); + assert(! stailq_empty(&s->event_queue)); struct swim_member *m = - stailq_shift_entry(&s->update_queue, struct swim_member, - in_update_queue); + stailq_shift_entry(&s->event_queue, struct swim_member, + in_event_queue); /* * It is possible, that a member was added and * removed before firing a trigger. It happens, @@ -1783,7 +1783,7 @@ swim_worker_f(va_list va) ctx.member = m; ctx.events = m->events; m->events = 0; - if (trigger_run(&s->on_member_update, (void *) &ctx)) + if (trigger_run(&s->on_member_event, (void *) &ctx)) diag_log(); } swim_member_unref(m); @@ -1832,8 +1832,8 @@ swim_new(void) /* Dissemination component. */ rlist_create(&swim->dissemination_queue); - rlist_create(&swim->on_member_update); - stailq_create(&swim->update_queue); + rlist_create(&swim->on_member_event); + stailq_create(&swim->event_queue); fiber_start(swim->worker, swim); return swim; @@ -2096,7 +2096,7 @@ swim_do_delete(struct swim *swim) swim_ev_timer_stop(l, &swim->round_tick); swim_ev_timer_stop(l, &swim->wait_ack_tick); struct swim_member *m, *tmp; - stailq_foreach_entry_safe(m, tmp, &swim->update_queue, in_update_queue) + stailq_foreach_entry_safe(m, tmp, &swim->event_queue, in_event_queue) swim_member_unref(m); mh_int_t node; mh_foreach(swim->members, node) { @@ -2114,7 +2114,7 @@ swim_do_delete(struct swim *swim) swim_task_destroy(&swim->round_step_task); wait_ack_heap_destroy(&swim->wait_ack_heap); mh_swim_table_delete(swim->members); - trigger_destroy(&swim->on_member_update); + trigger_destroy(&swim->on_member_event); free(swim->shuffled); free(swim); } diff --git a/src/lib/swim/swim.h b/src/lib/swim/swim.h index 8a5bc9522..a42ace7c6 100644 --- a/src/lib/swim/swim.h +++ b/src/lib/swim/swim.h @@ -286,8 +286,8 @@ enum swim_ev_mask { SWIM_EV_DROP = 0b00100000, }; -/** On member update trigger context. */ -struct swim_on_member_update_ctx { +/** On member event trigger context. */ +struct swim_on_member_event_ctx { /** New, dropped, or updated member. */ struct swim_member *member; /** Mask of happened events. */ @@ -295,22 +295,22 @@ struct swim_on_member_update_ctx { }; /** - * A list of member update triggers. A couple of words about such - * a strange API, instead of something like "add_trigger", - * "drop_trigger". A main motivation is that some functions need - * a whole trigger list. For example, a function adding Lua - * functions as triggers. At the time of this writing it was - * lbox_trigger_reset. There is a convention, that any Tarantool - * trigger exposed to Lua should provide a way to add one, drop - * one, replace one, return all - lbox_trigger_reset does all of - * this. + * A list of member event processing triggers. A couple of words + * about such a strange API, instead of something like + * "add_trigger", "drop_trigger". A main motivation is that some + * functions need a whole trigger list. For example, a function + * adding Lua functions as triggers. At the time of this writing + * it was lbox_trigger_reset. There is a convention, that any + * Tarantool trigger exposed to Lua should provide a way to add + * one, drop one, replace one, return all - lbox_trigger_reset + * does all of this. */ struct rlist * -swim_trigger_list_on_member_update(struct swim *swim); +swim_trigger_list_on_member_event(struct swim *swim); /** - * Check if a SWIM instance has pending update events. Is not a - * public one, used by tests. + * Check if a SWIM instance has pending events. Is not a public + * one, used by tests. */ bool swim_has_pending_events(struct swim *swim); diff --git a/test/unit/swim.c b/test/unit/swim.c index 3f2d156c5..8bf9b0eba 100644 --- a/test/unit/swim.c +++ b/test/unit/swim.c @@ -956,22 +956,22 @@ struct trigger_ctx { bool is_deleted; bool need_sleep; struct fiber *f; - struct swim_on_member_update_ctx ctx; + struct swim_on_member_event_ctx ctx; }; static void -swim_on_member_update_save_event(struct trigger *t, void *event) +swim_on_member_event_save(struct trigger *t, void *event) { struct trigger_ctx *c = (struct trigger_ctx *) t->data; ++c->counter; if (c->ctx.member != NULL) swim_member_unref(c->ctx.member); - c->ctx = *((struct swim_on_member_update_ctx *) event); + c->ctx = *((struct swim_on_member_event_ctx *) event); swim_member_ref(c->ctx.member); } static void -swim_on_member_update_yield(struct trigger *t, void *event) +swim_on_member_event_yield(struct trigger *t, void *event) { struct trigger_ctx *c = (struct trigger_ctx *) t->data; ++c->counter; @@ -997,14 +997,14 @@ swim_test_triggers(void) memset(&tctx2, 0, sizeof(tctx2)); struct trigger *t1 = (struct trigger *) malloc(sizeof(*t1)); assert(t1 != NULL); - trigger_create(t1, swim_on_member_update_save_event, (void *) &tctx, + trigger_create(t1, swim_on_member_event_save, (void *) &tctx, swim_trigger_destroy_cb); /* Skip 'new self' events. */ swim_cluster_run_triggers(cluster); struct swim *s1 = swim_cluster_member(cluster, 0); - trigger_add(swim_trigger_list_on_member_update(s1), t1); + trigger_add(swim_trigger_list_on_member_event(s1), t1); swim_cluster_interconnect(cluster, 0, 1); swim_cluster_run_triggers(cluster); @@ -1052,14 +1052,14 @@ swim_test_triggers(void) * There is a complication about yields. If a trigger * yields, other triggers wait for its finish. And all * the triggers should be ready to SWIM deletion in the - * middle of an update processing. SWIM object should not + * middle of an event processing. SWIM object should not * be deleted, until all the triggers are done. */ struct trigger *t2 = (struct trigger *) malloc(sizeof(*t2)); assert(t2 != NULL); tctx2.need_sleep = true; - trigger_create(t2, swim_on_member_update_yield, (void *) &tctx2, NULL); - trigger_add(swim_trigger_list_on_member_update(s1), t2); + trigger_create(t2, swim_on_member_event_yield, (void *) &tctx2, NULL); + trigger_add(swim_trigger_list_on_member_event(s1), t2); swim_cluster_add_link(cluster, 0, 1); swim_cluster_run_triggers(cluster); is(tctx2.counter, 1, "yielding trigger is fired"); diff --git a/test/unit/swim_test_utils.c b/test/unit/swim_test_utils.c index 0b4743b2d..463c62390 100644 --- a/test/unit/swim_test_utils.c +++ b/test/unit/swim_test_utils.c @@ -200,8 +200,8 @@ void swim_test_event_cb(struct trigger *trigger, void *event) { (void) trigger; - struct swim_on_member_update_ctx *ctx = - (struct swim_on_member_update_ctx *) event; + struct swim_on_member_event_ctx *ctx = + (struct swim_on_member_event_ctx *) event; assert(ctx->events != 0); assert((ctx->events & SWIM_EV_NEW) == 0 || (ctx->events & SWIM_EV_DROP) == 0); @@ -216,7 +216,7 @@ swim_node_create(struct swim_node *n, int id) assert(n->swim != NULL); struct trigger *t = (struct trigger *) malloc(sizeof(*t)); trigger_create(t, swim_test_event_cb, NULL, (trigger_f0) free); - trigger_add(swim_trigger_list_on_member_update(n->swim), t); + trigger_add(swim_trigger_list_on_member_event(n->swim), t); char uri[128]; swim_cluster_id_to_uri(uri, id); ======================================================================= >> + /** >> + * Worker fiber to serve member update triggers. This task >> + * is being done in a separate fiber, because user >> + * triggers can yield and libev callbacks, processing >> + * member updates, are not allowed to yield. >> + */ >> + struct fiber *worker; > > I think as long as the worker is only used for triggers, the name > should be more specific. when the worker is used for other events > or actions, it could be renamed. Perhaps. ======================================================================= diff --git a/src/lib/swim/swim.c b/src/lib/swim/swim.c index d48e43761..fbc3cc7be 100644 --- a/src/lib/swim/swim.c +++ b/src/lib/swim/swim.c @@ -498,12 +498,12 @@ struct swim { */ struct swim_member **shuffled; /** - * Worker fiber to serve member event triggers. This task - * is being done in a separate fiber, because user - * triggers can yield and libev callbacks, processing - * member events, are not allowed to yield. + * Fiber to serve member event triggers. This task is + * being done in a separate fiber, because user triggers + * can yield and libev callbacks, processing member + * events, are not allowed to yield. */ - struct fiber *worker; + struct fiber *event_handler; /** * Single round step task. It is impossible to have * multiple round steps in the same SWIM instance at the @@ -600,7 +600,7 @@ swim_on_member_update(struct swim *swim, struct swim_member *member, swim_member_ref(member); stailq_add_tail_entry(&swim->event_queue, member, in_event_queue); - fiber_wakeup(swim->worker); + fiber_wakeup(swim->event_handler); } member->events |= events; } @@ -1736,14 +1736,14 @@ error: /** * Do actual deletion of a SWIM instance. This destructor is - * called only after worker fiber has been stopped, and triggers - * do not work. + * called only after event handler fiber has been stopped, and + * triggers do not work. */ static void swim_do_delete(struct swim *swim); /** - * Worker fiber. At this moment its only task is dispatching + * Event handler. At this moment its only task is dispatching * member events to user defined triggers. Generally, because * SWIM is fully IO driven, that fiber should be used only for * yielding tasks not related to SWIM core logic. For all the @@ -1752,7 +1752,7 @@ swim_do_delete(struct swim *swim); * invoked by a cord scheduler fiber prohibited for manual yields. */ static int -swim_worker_f(va_list va) +swim_event_handler_f(va_list va) { struct swim *s = va_arg(va, struct swim *); struct swim_on_member_event_ctx ctx; @@ -1802,14 +1802,15 @@ swim_new(void) diag_set(OutOfMemory, sizeof(*swim), "calloc", "swim"); return NULL; } - swim->worker = fiber_new("SWIM worker", swim_worker_f); - if (swim->worker == NULL) { + swim->event_handler = fiber_new("SWIM event handler", + swim_event_handler_f); + if (swim->event_handler == NULL) { free(swim); return NULL; } swim->members = mh_swim_table_new(); if (swim->members == NULL) { - fiber_cancel(swim->worker); + fiber_cancel(swim->event_handler); free(swim); diag_set(OutOfMemory, sizeof(*swim->members), "mh_swim_table_new", "members"); @@ -1835,7 +1836,7 @@ swim_new(void) rlist_create(&swim->on_member_event); stailq_create(&swim->event_queue); - fiber_start(swim->worker, swim); + fiber_start(swim->event_handler, swim); return swim; } @@ -1944,8 +1945,9 @@ swim_cfg(struct swim *swim, const char *uri, double heartbeat_rate, * specified. */ addr = swim->scheduler.transport.addr; - fiber_set_name(swim->worker, tt_sprintf("SWIM worker %d", - swim_fd(swim))); + fiber_set_name(swim->event_handler, + tt_sprintf("SWIM event handler %d", + swim_fd(swim))); } else { addr = swim->self->addr; } @@ -2122,8 +2124,8 @@ swim_do_delete(struct swim *swim) void swim_delete(struct swim *swim) { - fiber_wakeup(swim->worker); - fiber_cancel(swim->worker); + fiber_wakeup(swim->event_handler); + fiber_cancel(swim->event_handler); fiber_sleep(0); } @@ -2139,8 +2141,8 @@ swim_quit_step_complete(struct swim_task *task, (void) rc; struct swim *swim = swim_by_scheduler(scheduler); if (rlist_empty(&swim->round_queue)) { - fiber_wakeup(swim->worker); - fiber_cancel(swim->worker); + fiber_wakeup(swim->event_handler); + fiber_cancel(swim->event_handler); return; } struct swim_member *m = ======================================================================= > >> @@ -734,7 +801,6 @@ swim_delete_member(struct swim *swim, struct swim_member *member) >> mh_int_t rc = mh_swim_table_find(swim->members, key, NULL); >> assert(rc != mh_end(swim->members)); >> mh_swim_table_del(swim->members, rc, NULL); >> - swim_cached_round_msg_invalidate(swim); >> rlist_del_entry(member, in_round_queue); > > Stray change? At least I don't understand it. No, it is not stray. I've started calling swim_on_member_update() in swim_delete_member(). The former calls swim_cached_round_msg_invalidate too, so I decided to remove one from the latter. > >> + struct swim *s = va_arg(va, struct swim *); >> + struct swim_on_member_update_ctx ctx; >> + while (! fiber_is_cancelled()) { >> + while (stailq_empty(&s->update_queue)) { >> + fiber_yield(); >> + if (fiber_is_cancelled()) >> + goto end; > >> + } > > Since you use goto anyway, why a double loop? > > while (! fiber_is_cancelled()) { > if (stailq_empty(&s->update_queue) { > fiber_yield(); > continue; > } > ... > } You are right. I forgot about 'continue', shame on me. ======================================================================= diff --git a/src/lib/swim/swim.c b/src/lib/swim/swim.c index fbc3cc7be..17e1e6a7b 100644 --- a/src/lib/swim/swim.c +++ b/src/lib/swim/swim.c @@ -1757,10 +1757,9 @@ swim_event_handler_f(va_list va) struct swim *s = va_arg(va, struct swim *); struct swim_on_member_event_ctx ctx; while (! fiber_is_cancelled()) { - while (stailq_empty(&s->event_queue)) { + if (stailq_empty(&s->event_queue)) { fiber_yield(); - if (fiber_is_cancelled()) - goto end; + continue; } /* * Can't be empty. SWIM deletes members from @@ -1788,7 +1787,6 @@ swim_event_handler_f(va_list va) } swim_member_unref(m); } -end: swim_do_delete(s); return 0; } ======================================================================= >> + } >> +end: >> + swim_do_delete(s); > > I don't mind swim_do_delete(), but I think the worker should not > be destorying the instance. Looks a breach of encapsulation, this assumes > the workers knows about all other uses of struct swim. > > Why not cancel and join the worker from the destructor and then > safely do the delete? Yes, this is a breach. My motivation why I did this after multiple iterations of patch remake and lots of doubts was that I wanted swim:delete() be fast. With fiber_join() it will wait while all the triggers are finished. But probably I magnified distress as usual, and fiber_join would be really much simpler here. ======================================================================= diff --git a/src/lib/swim/swim.c b/src/lib/swim/swim.c index 17e1e6a7b..6f34faa71 100644 --- a/src/lib/swim/swim.c +++ b/src/lib/swim/swim.c @@ -1734,14 +1734,6 @@ error: diag_log(); } -/** - * Do actual deletion of a SWIM instance. This destructor is - * called only after event handler fiber has been stopped, and - * triggers do not work. - */ -static void -swim_do_delete(struct swim *swim); - /** * Event handler. At this moment its only task is dispatching * member events to user defined triggers. Generally, because @@ -1787,7 +1779,6 @@ swim_event_handler_f(va_list va) } swim_member_unref(m); } - swim_do_delete(s); return 0; } @@ -1806,6 +1797,7 @@ swim_new(void) free(swim); return NULL; } + fiber_set_joinable(swim->event_handler, true); swim->members = mh_swim_table_new(); if (swim->members == NULL) { fiber_cancel(swim->event_handler); @@ -2088,9 +2080,34 @@ swim_size(const struct swim *swim) return mh_size(swim->members); } -static void -swim_do_delete(struct swim *swim) +/** + * Cancel and wait finish of an event handler fiber. That + * operation is not inlined in the SWIM destructor, because there + * is one more place, when the handler should be stopped even + * before SWIM deletion - quit. Quit deletes the instance only + * when all the 'I left' messages are sent, but it happens in a + * libev callback in the scheduler fiber where it is impossible + * to yield. So to that moment the handler should be dead already. + */ +static inline void +swim_kill_event_handler(struct swim *swim) { + struct fiber *f = swim->event_handler; + /* + * Nullify so as not to keep pointer at a fiber when it is + * reused. + */ + swim->event_handler = NULL; + fiber_wakeup(f); + fiber_cancel(f); + fiber_join(f); +} + +void +swim_delete(struct swim *swim) +{ + if (swim->event_handler != NULL) + swim_kill_event_handler(swim); struct ev_loop *l = swim_loop(); swim_scheduler_destroy(&swim->scheduler); swim_ev_timer_stop(l, &swim->round_tick); @@ -2119,14 +2136,6 @@ swim_do_delete(struct swim *swim) free(swim); } -void -swim_delete(struct swim *swim) -{ - fiber_wakeup(swim->event_handler); - fiber_cancel(swim->event_handler); - fiber_sleep(0); -} - /** * Quit message is broadcasted in the same way as round messages, * step by step, with the only difference that quit round steps @@ -2139,8 +2148,12 @@ swim_quit_step_complete(struct swim_task *task, (void) rc; struct swim *swim = swim_by_scheduler(scheduler); if (rlist_empty(&swim->round_queue)) { - fiber_wakeup(swim->event_handler); - fiber_cancel(swim->event_handler); + /* + * The handler should be dead - can't yield here, + * it is the scheduler fiber. + */ + assert(swim->event_handler == NULL); + swim_delete(swim); return; } struct swim_member *m = @@ -2169,6 +2182,11 @@ void swim_quit(struct swim *swim) { assert(swim_is_configured(swim)); + /* + * Kill the handler now. Later it will be impossible to do + * from the scheduler fiber. + */ + swim_kill_event_handler(swim); struct ev_loop *l = swim_loop(); swim_ev_timer_stop(l, &swim->round_tick); swim_ev_timer_stop(l, &swim->wait_ack_tick); diff --git a/test/unit/swim.c b/test/unit/swim.c index 8bf9b0eba..0e33d691c 100644 --- a/test/unit/swim.c +++ b/test/unit/swim.c @@ -986,6 +986,15 @@ swim_trigger_destroy_cb(struct trigger *t) ((struct trigger_ctx *) t->data)->is_deleted = true; } +static int +swim_cluster_delete_f(va_list ap) +{ + struct swim_cluster *c = (struct swim_cluster *) + va_arg(ap, struct swim_cluster *); + swim_cluster_delete(c); + return 0; +} + static void swim_test_triggers(void) { @@ -1065,7 +1074,9 @@ swim_test_triggers(void) is(tctx2.counter, 1, "yielding trigger is fired"); is(tctx.counter, 6, "non-yielding still is not"); - swim_cluster_delete(cluster); + struct fiber *async_delete_fiber = + fiber_new("async delete", swim_cluster_delete_f); + fiber_start(async_delete_fiber, cluster); ok(! tctx.is_deleted, "trigger is not deleted until all currently "\ "sleeping triggers are finished"); tctx2.need_sleep = false; ======================================================================= >> + swim->worker = fiber_new("SWIM worker", swim_worker_f); >> + if (swim->worker == NULL) { >> + free(swim); >> + return NULL; >> + } > >> swim->members = mh_swim_table_new(); >> if (swim->members == NULL) { >> + fiber_cancel(swim->worker); > > Better to start the worker last, then you will have easy time > destroying it on failure. This is a minor nit, ignore if you wish. ======================================================================= diff --git a/src/lib/swim/swim.c b/src/lib/swim/swim.c index ae66ce198..00234d1df 100644 --- a/src/lib/swim/swim.c +++ b/src/lib/swim/swim.c @@ -1791,15 +1791,8 @@ swim_new(void) diag_set(OutOfMemory, sizeof(*swim), "calloc", "swim"); return NULL; } - swim->event_handler = fiber_new("SWIM event handler", - swim_event_handler_f); - if (swim->event_handler == NULL) { - free(swim); - return NULL; - } swim->members = mh_swim_table_new(); if (swim->members == NULL) { - fiber_cancel(swim->event_handler); free(swim); diag_set(OutOfMemory, sizeof(*swim->members), "mh_swim_table_new", "members"); @@ -1824,7 +1817,13 @@ swim_new(void) rlist_create(&swim->dissemination_queue); rlist_create(&swim->on_member_event); stailq_create(&swim->event_queue); - + swim->event_handler = fiber_new("SWIM event handler", + swim_event_handler_f); + if (swim->event_handler == NULL) { + swim_delete(swim); + return NULL; + } + fiber_set_joinable(swim->event_handler, true); fiber_start(swim->event_handler, swim); return swim; }