Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@freelists.org
Cc: kostja@tarantool.org
Subject: [tarantool-patches] [PATCH 1/5] test: create isolated ev_loop for swim unit tests
Date: Sun,  2 Jun 2019 02:00:17 +0200	[thread overview]
Message-ID: <50a84d1ffdd25646894e576ef4ff19195aaf769e.1559433539.git.v.shpilevoy@tarantool.org> (raw)
In-Reply-To: <cover.1559433539.git.v.shpilevoy@tarantool.org>

The SWIM unit tests code with the fake events and time does lots
of forbidden things: it manually invokes pending watcher
callbacks; manages global time without a kernel; puts not
existing descriptors into the loop. These foul blows open the
gates to the full control over IO events, descriptors, virtual
time. Hundreds of virtual seconds pass in milliseconds in
reality, it makes SWIM unit tests fast despite complex logic.

All these actions does not affect the loop until yield. On yield
a scheduler fiber wakes up and

    1) infinitely generates EV_READ on not existing descriptors
       because a kernel considers them closed;

    2) manual pending callbacks invocation asserts, because it is
       not allowed for non-scheduler fibers.

To avoid these problems a new isolated loop is created, not
visible for the scheduler. Here the fake events library can rack
and ruin whatever it wants.

Needed for #4250
---
 src/lib/swim/swim.c         | 21 ++++++++++++---------
 src/lib/swim/swim_ev.c      |  6 ++++++
 src/lib/swim/swim_ev.h      |  3 +++
 src/lib/swim/swim_io.c      | 15 ++++++++-------
 test/unit/swim_test_ev.c    | 24 ++++++++++++++++++++++++
 test/unit/swim_test_utils.c |  2 +-
 6 files changed, 54 insertions(+), 17 deletions(-)

diff --git a/src/lib/swim/swim.c b/src/lib/swim/swim.c
index f7a885b76..46b76731d 100644
--- a/src/lib/swim/swim.c
+++ b/src/lib/swim/swim.c
@@ -519,7 +519,7 @@ swim_wait_ack(struct swim *swim, struct swim_member *member,
 			timeout *= 2;
 		member->ping_deadline = swim_time() + timeout;
 		wait_ack_heap_insert(&swim->wait_ack_heap, member);
-		swim_ev_timer_again(loop(), &swim->wait_ack_tick);
+		swim_ev_timer_again(swim_loop(), &swim->wait_ack_tick);
 	}
 }
 
@@ -802,7 +802,7 @@ swim_new_member(struct swim *swim, const struct sockaddr_in *addr,
 		return NULL;
 	}
 	if (mh_size(swim->members) > 1)
-		swim_ev_timer_again(loop(), &swim->round_tick);
+		swim_ev_timer_again(swim_loop(), &swim->round_tick);
 
 	/* Dissemination component. */
 	swim_on_member_update(swim, member);
@@ -1122,7 +1122,7 @@ swim_complete_step(struct swim_task *task,
 	 * It could be stopped by the step begin function, if the
 	 * sending was too long.
 	 */
-	swim_ev_timer_again(loop(), &swim->round_tick);
+	swim_ev_timer_again(swim_loop(), &swim->round_tick);
 	/*
 	 * It is possible that the original member was deleted
 	 * manually during the task execution.
@@ -1813,16 +1813,17 @@ swim_cfg(struct swim *swim, const char *uri, double heartbeat_rate,
 		addr = swim->self->addr;
 	}
 	struct ev_timer *t = &swim->round_tick;
+	struct ev_loop *l = swim_loop();
 	if (t->repeat != heartbeat_rate && heartbeat_rate > 0) {
 		swim_ev_timer_set(t, 0, heartbeat_rate);
 		if (swim_ev_is_active(t))
-			swim_ev_timer_again(loop(), t);
+			swim_ev_timer_again(l, t);
 	}
 	t = &swim->wait_ack_tick;
 	if (t->repeat != ack_timeout && ack_timeout > 0) {
 		swim_ev_timer_set(t, 0, ack_timeout);
 		if (swim_ev_is_active(t))
-			swim_ev_timer_again(loop(), t);
+			swim_ev_timer_again(l, t);
 	}
 
 	if (new_self != NULL) {
@@ -1953,9 +1954,10 @@ swim_size(const struct swim *swim)
 void
 swim_delete(struct swim *swim)
 {
+	struct ev_loop *l = swim_loop();
 	swim_scheduler_destroy(&swim->scheduler);
-	swim_ev_timer_stop(loop(), &swim->round_tick);
-	swim_ev_timer_stop(loop(), &swim->wait_ack_tick);
+	swim_ev_timer_stop(l, &swim->round_tick);
+	swim_ev_timer_stop(l, &swim->wait_ack_tick);
 	mh_int_t node;
 	mh_foreach(swim->members, node) {
 		struct swim_member *m =
@@ -2018,8 +2020,9 @@ void
 swim_quit(struct swim *swim)
 {
 	assert(swim_is_configured(swim));
-	swim_ev_timer_stop(loop(), &swim->round_tick);
-	swim_ev_timer_stop(loop(), &swim->wait_ack_tick);
+	struct ev_loop *l = swim_loop();
+	swim_ev_timer_stop(l, &swim->round_tick);
+	swim_ev_timer_stop(l, &swim->wait_ack_tick);
 	swim_scheduler_stop_input(&swim->scheduler);
 	/* Start the last round - quiting. */
 	swim_new_round(swim);
diff --git a/src/lib/swim/swim_ev.c b/src/lib/swim/swim_ev.c
index 49c8c273b..82668d41d 100644
--- a/src/lib/swim/swim_ev.c
+++ b/src/lib/swim/swim_ev.c
@@ -55,3 +55,9 @@ swim_ev_timer_stop(struct ev_loop *loop, struct ev_timer *watcher)
 {
 	ev_timer_stop(loop, watcher);
 }
+
+struct ev_loop *
+swim_loop(void)
+{
+	return loop();
+}
diff --git a/src/lib/swim/swim_ev.h b/src/lib/swim/swim_ev.h
index 1bd81306f..900be150f 100644
--- a/src/lib/swim/swim_ev.h
+++ b/src/lib/swim/swim_ev.h
@@ -52,6 +52,9 @@ swim_ev_timer_again(struct ev_loop *loop, struct ev_timer *watcher);
 void
 swim_ev_timer_stop(struct ev_loop *loop, struct ev_timer *watcher);
 
+struct ev_loop *
+swim_loop(void);
+
 #define swim_ev_is_active ev_is_active
 
 #define swim_ev_init ev_init
diff --git a/src/lib/swim/swim_io.c b/src/lib/swim/swim_io.c
index c55c276cb..e7ff321d4 100644
--- a/src/lib/swim/swim_io.c
+++ b/src/lib/swim/swim_io.c
@@ -148,7 +148,7 @@ swim_task_schedule(struct swim_task *task, struct swim_scheduler *scheduler)
 {
 	assert(! swim_task_is_scheduled(task));
 	rlist_add_tail_entry(&scheduler->queue_output, task, in_queue_output);
-	swim_ev_io_start(loop(), &scheduler->output);
+	swim_ev_io_start(swim_loop(), &scheduler->output);
 }
 
 void
@@ -289,16 +289,17 @@ int
 swim_scheduler_bind(struct swim_scheduler *scheduler,
 		    const struct sockaddr_in *addr)
 {
-	swim_ev_io_stop(loop(), &scheduler->input);
-	swim_ev_io_stop(loop(), &scheduler->output);
+	struct ev_loop *l = swim_loop();
+	swim_ev_io_stop(l, &scheduler->input);
+	swim_ev_io_stop(l, &scheduler->output);
 	struct swim_transport *t = &scheduler->transport;
 	int rc = swim_transport_bind(t, (const struct sockaddr *) addr,
 				     sizeof(*addr));
 	if (t->fd >= 0) {
 		swim_ev_io_set(&scheduler->output, t->fd, EV_WRITE);
 		swim_ev_io_set(&scheduler->input, t->fd, EV_READ);
-		swim_ev_io_start(loop(), &scheduler->input);
-		swim_ev_io_start(loop(), &scheduler->output);
+		swim_ev_io_start(l, &scheduler->input);
+		swim_ev_io_start(l, &scheduler->output);
 	}
 	return rc;
 }
@@ -306,7 +307,7 @@ swim_scheduler_bind(struct swim_scheduler *scheduler,
 void
 swim_scheduler_stop_input(struct swim_scheduler *scheduler)
 {
-	swim_ev_io_stop(loop(), &scheduler->input);
+	swim_ev_io_stop(swim_loop(), &scheduler->input);
 }
 
 void
@@ -323,7 +324,7 @@ swim_scheduler_destroy(struct swim_scheduler *scheduler)
 			t->cancel(t, scheduler, -1);
 	}
 	swim_transport_destroy(&scheduler->transport);
-	swim_ev_io_stop(loop(), &scheduler->output);
+	swim_ev_io_stop(swim_loop(), &scheduler->output);
 	swim_scheduler_stop_input(scheduler);
 }
 
diff --git a/test/unit/swim_test_ev.c b/test/unit/swim_test_ev.c
index a4ffa2fc8..fb25ac9e4 100644
--- a/test/unit/swim_test_ev.c
+++ b/test/unit/swim_test_ev.c
@@ -62,6 +62,27 @@ struct swim_event;
 typedef void (*swim_event_process_f)(struct swim_event *, struct ev_loop *);
 typedef void (*swim_event_delete_f)(struct swim_event *);
 
+/**
+ * The unit tests code with the fake events and time does lots of
+ * forbidden things: it manually invokes pending watcher
+ * callbacks; manages global time without a kernel; puts not
+ * existing descriptors into the loop. All these actions does not
+ * affect the loop until yield. On yield a scheduler fiber wakes
+ * up and 1) infinitely generates EV_READ on not existing
+ * descriptors because considers them closed; 2) manual pending
+ * callbacks invocation asserts, because it is not allowed for
+ * non-scheduler fibers. To avoid these problems a new isolated
+ * loop is created, not visible for the scheduler. Here the fake
+ * events library can rack and ruin whatever it wants.
+ */
+static struct ev_loop *test_loop;
+
+struct ev_loop *
+swim_loop(void)
+{
+	return test_loop;
+}
+
 /**
  * Base event. It is stored in the event heap and virtualizes
  * other events.
@@ -330,6 +351,8 @@ swim_test_ev_init(void)
 	events_hash = mh_i64ptr_new();
 	assert(events_hash != NULL);
 	event_heap_create(&event_heap);
+	test_loop = ev_loop_new(0);
+	assert(test_loop != NULL);
 }
 
 void
@@ -338,4 +361,5 @@ swim_test_ev_free(void)
 	swim_test_ev_reset();
 	event_heap_destroy(&event_heap);
 	mh_i64ptr_delete(events_hash);
+	ev_loop_destroy(test_loop);
 }
diff --git a/test/unit/swim_test_utils.c b/test/unit/swim_test_utils.c
index ffd42cbd0..f72fa2450 100644
--- a/test/unit/swim_test_utils.c
+++ b/test/unit/swim_test_utils.c
@@ -607,7 +607,7 @@ swim_wait_timeout(double timeout, struct swim_cluster *cluster,
 {
 	swim_ev_set_brk(timeout);
 	double deadline = swim_time() + timeout;
-	struct ev_loop *loop = loop();
+	struct ev_loop *loop = swim_loop();
 	/*
 	 * There can be pending out of bound IO events, affecting
 	 * the result. For example, 'quit' messages, which are
-- 
2.20.1 (Apple Git-117)

  reply	other threads:[~2019-06-02  0:00 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-02  0:00 [tarantool-patches] [PATCH 0/5] SWIM on_member_update Vladislav Shpilevoy
2019-06-02  0:00 ` Vladislav Shpilevoy [this message]
2019-06-05  6:51   ` [tarantool-patches] Re: [PATCH 1/5] test: create isolated ev_loop for swim unit tests Konstantin Osipov
2019-06-05 21:53     ` Vladislav Shpilevoy
2019-06-08 14:24       ` Konstantin Osipov
2019-06-02  0:00 ` [tarantool-patches] [PATCH 2/5] swim: fix a 'use after free' in SWIM tests Vladislav Shpilevoy
2019-06-05  6:52   ` [tarantool-patches] " Konstantin Osipov
2019-06-02  0:00 ` [tarantool-patches] [PATCH 3/5] swim: allow to hang triggers on member updates Vladislav Shpilevoy
2019-06-05  7:11   ` [tarantool-patches] " Konstantin Osipov
2019-06-05 21:53     ` Vladislav Shpilevoy
2019-06-07 13:35       ` Konstantin Osipov
2019-06-02  0:00 ` [tarantool-patches] [PATCH 4/5] swim: call swim:new/delete via Lua C, not via FFI Vladislav Shpilevoy
2019-06-08 14:24   ` [tarantool-patches] " Konstantin Osipov
2019-06-02  0:10 ` [tarantool-patches] [PATCH 5/5] swim: expose Lua triggers on member update Vladislav Shpilevoy
2019-06-05 21:54   ` [tarantool-patches] " Vladislav Shpilevoy
2019-06-08 14:29     ` Konstantin Osipov
     [not found] ` <12b8ea76f7c1cd100a80ddcea3c29d20354e073e.1559433539.git.v.shpilevoy@tarantool.org>
2019-06-08 14:27   ` Konstantin Osipov
2019-06-08 19:52     ` Vladislav Shpilevoy
2019-06-09  5:15       ` Konstantin Osipov
2019-06-09 16:41         ` Vladislav Shpilevoy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50a84d1ffdd25646894e576ef4ff19195aaf769e.1559433539.git.v.shpilevoy@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=kostja@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [tarantool-patches] [PATCH 1/5] test: create isolated ev_loop for swim unit tests' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox