[patch] Patch to fix broken regress_http test

20-fix-test-free-active-base.diff (text/plain), 8.64 KB, created by Hans de Graaff on 2009-09-05 12:44:21 UTC

Convert test_free_active_base and test_event_base_new from legacy tests; rework test_free_active_base to not crash with a double free under some conditions

diff --git a/Makefile.am b/Makefile.am
--- a/Makefile.am
+++ b/Makefile.am
@@ -133,7 +133,8 @@ check_HEADERS = test/tinytest.h test/tin
 check_PROGRAMS = \
 	event-test time-test signal-test \
 	test-init test-eof test-weof test-time regress \
-	bench bench_cascade bench_http bench_httpclient
+	bench bench_cascade bench_http bench_httpclient \
+	tinytest_demo
 
 TESTS = $(srcdir)/test/test.sh
 
@@ -169,6 +170,8 @@ regress_SOURCES = test/regress.c test/re
 	test/regress_main.c test/regress_minheap.c
 regress_LDADD = libevent.la
 
+tinytest_demo_SOURCES = test/tinytest_demo.c test/tinytest.c
+
 if PTHREADS
 regress_SOURCES += test/regress_pthread.c
 regress_LDADD += libevent_pthreads.la
@@ -219,7 +222,7 @@ test/regress.gen.c test/regress.gen.h: t
 EXTRA_DIST = \
 	autogen.sh Doxyfile whatsnew-2.0.txt			\
 	test/regress.gen.c test/regress.gen.h test/regress.rpc	\
-	test/test.sh test/tinytest_demo.c			\
+	test/test.sh						\
 	WIN32-Code/config.h WIN32-Code/event-config.h		\
 	WIN32-Code/tree.h					\
 	WIN32-Prj/libevent.dsp WIN32-Prj/libevent.dsw		\
diff --git a/test/regress.c b/test/regress.c
--- a/test/regress.c
+++ b/test/regress.c
@@ -99,6 +99,13 @@ static struct timeval tcalled;
 #define read(fd,buf,len) recv((fd),(buf),(len),0)
 #endif
 
+struct basic_cb_args
+{
+	struct event_base *eb;
+	struct event *ev;
+	unsigned int callcount;
+};
+
 static void
 simple_read_cb(int fd, short event, void *arg)
 {
@@ -119,6 +126,45 @@ simple_read_cb(int fd, short event, void
 }
 
 static void
+basic_read_cb(int fd, short event, void *data)
+{
+	char buf[256];
+	int len;
+	struct basic_cb_args *arg = data;
+
+	len = read(fd, buf, sizeof(buf));
+
+	if (len < 0) {
+		tt_fail_perror("read (callback)");
+	} else {
+		switch (arg->callcount++) {
+		case 0:  /* first call: expect to read data; cycle */
+			if (len > 0)
+				return;
+
+			tt_fail_msg("EOF before data read");
+			break;
+
+		case 1:  /* second call: expect EOF; stop */
+			if (len > 0)
+				tt_fail_msg("not all data read on first cycle");
+			break;
+
+		default:  /* third call: should not happen */
+			tt_fail_msg("too many cycles");
+		}
+	}
+
+	event_del(arg->ev);
+	event_base_loopexit(arg->eb, NULL);
+}
+
+static void
+dummy_read_cb(int fd, short event, void *arg)
+{
+}
+
+static void
 simple_write_cb(int fd, short event, void *arg)
 {
 	int len;
@@ -893,43 +939,62 @@ test_signal_while_processing(void)
 #endif
 
 static void
-test_free_active_base(void)
+test_free_active_base(void *ptr)
 {
+	struct basic_test_data *data = ptr;
 	struct event_base *base1;
-	struct event ev1;
-	setup_test("Free active base: ");
+
 	base1 = event_init();
-	event_set(&ev1, pair[1], EV_READ, simple_read_cb, &ev1);
-	event_base_set(base1, &ev1);
-	event_add(&ev1, NULL);
-	/* event_del(&ev1); */
-	event_base_free(base1);
-	test_ok = 1;
-	cleanup_test();
-	event_base_free(global_base);
-	global_base = event_init();
+	if (base1) {
+		struct event ev1;
+		event_assign(&ev1, base1, data->pair[1], EV_READ,
+			     dummy_read_cb, NULL);
+		event_add(&ev1, NULL);
+		event_base_free(base1);  /* should not crash */
+	} else {
+		tt_fail_msg("failed to create event_base for test");
+	}
 }
 
 static void
-test_event_base_new(void)
+test_event_base_new(void *ptr)
 {
-	struct event_base *base;
+	struct basic_test_data *data = ptr;
+	struct event_base *base = 0;
 	struct event ev1;
-	setup_test("Event base new: ");
+	struct basic_cb_args args;
 
-	write(pair[0], TEST1, strlen(TEST1)+1);
-	shutdown(pair[0], SHUT_WR);
+	int towrite = strlen(TEST1)+1;
+	int len = write(data->pair[0], TEST1, towrite);
+
+	if (len < 0) 
+		tt_abort_perror("initial write");
+	else if (len != towrite)
+		tt_abort_printf(("initial write fell short (%d of %d bytes)",
+				 len, towrite));
+			
+	if (shutdown(data->pair[0], SHUT_WR))
+		tt_abort_perror("initial write shutdown");
 
 	base = event_base_new();
-	event_set(&ev1, pair[1], EV_READ, simple_read_cb, &ev1);
-	event_base_set(base, &ev1);
-	event_add(&ev1, NULL);
+	if (!base)
+		tt_abort_msg("failed to create event base");
 
-	event_base_dispatch(base);
+	args.eb = base;
+	args.ev = &ev1;
+	args.callcount = 0;
+	event_assign(&ev1, base, data->pair[1],
+		     EV_READ|EV_PERSIST, basic_read_cb, &args);
 
-	event_base_free(base);
-	test_ok = 1;
-	cleanup_test();
+	if (event_add(&ev1, NULL))
+		tt_abort_perror("initial event_add");
+
+	if (event_base_loop(base, 0))
+		tt_abort_msg("unsuccessful exit from event loop");
+
+end:
+	if (base)
+		event_base_free(base);
 }
 
 static void
@@ -1484,6 +1549,7 @@ test_base_environ(void *arg)
 	tt_assert(base);
 
 	defaultname = event_base_get_method(base);
+	TT_BLATHER(("default is <%s>", defaultname));
 	event_base_free(base);
 	base = NULL;
 
@@ -1676,13 +1742,13 @@ struct testcase_t main_testcases[] = {
 	{ "base_features", test_base_features, TT_FORK, NULL, NULL },
 	{ "base_environ", test_base_environ, TT_FORK, NULL, NULL },
 
+	BASIC(event_base_new, TT_FORK|TT_NEED_SOCKETPAIR),
+	BASIC(free_active_base, TT_FORK|TT_NEED_SOCKETPAIR),
+
         /* These are still using the old API */
         LEGACY(persistent_timeout, TT_FORK|TT_NEED_BASE),
         LEGACY(priorities, TT_FORK|TT_NEED_BASE),
 
-        LEGACY(free_active_base, TT_FORK|TT_NEED_BASE|TT_NEED_SOCKETPAIR),
-        LEGACY(event_base_new, TT_FORK|TT_NEED_SOCKETPAIR),
-
         /* These legacy tests may not all need all of these flags. */
         LEGACY(simpleread, TT_ISOLATED),
         LEGACY(simpleread_multiple, TT_ISOLATED),
diff --git a/test/regress.h b/test/regress.h
--- a/test/regress.h
+++ b/test/regress.h
@@ -79,6 +79,9 @@ void run_legacy_test_fn(void *ptr);
 /* All the flags that a legacy test needs. */
 #define TT_ISOLATED TT_FORK|TT_NEED_SOCKETPAIR|TT_NEED_BASE
 
+#define BASIC(name,flags)						\
+	{ #name, test_## name, flags, &basic_setup, NULL }
+
 #define LEGACY(name,flags)						\
 	{ #name, run_legacy_test_fn, flags|TT_LEGACY, &legacy_setup,	\
 	  test_## name }
diff --git a/test/regress_dns.c b/test/regress_dns.c
--- a/test/regress_dns.c
+++ b/test/regress_dns.c
@@ -356,7 +356,7 @@ dns_server(void)
 	/* Now configure a nameserver port. */
 	sock = socket(AF_INET, SOCK_DGRAM, 0);
         if (sock<=0) {
-                tt_fail_perror("socket");
+                tt_abort_perror("socket");
         }
 
         evutil_make_socket_nonblocking(sock);
@@ -366,7 +366,7 @@ dns_server(void)
 	my_addr.sin_port = htons(35353);
 	my_addr.sin_addr.s_addr = htonl(0x7f000001UL);
 	if (bind(sock, (struct sockaddr*)&my_addr, sizeof(my_addr)) < 0) {
-		tt_fail_perror("bind");
+		tt_abort_perror("bind");
 	}
 	port = evdns_add_server_port(sock, 0, dns_server_request_cb, NULL);
 
diff --git a/test/regress_et.c b/test/regress_et.c
--- a/test/regress_et.c
+++ b/test/regress_et.c
@@ -90,7 +90,7 @@ test_edgetriggered(void *et)
 	int success;
 
 	if (evutil_socketpair(LOCAL_SOCKETPAIR_AF, SOCK_STREAM, 0, pair) == -1) {
-		tt_fail_perror("socketpair");
+		tt_abort_perror("socketpair");
 	}
 
 	called = was_et = 0;
diff --git a/test/regress_zlib.c b/test/regress_zlib.c
--- a/test/regress_zlib.c
+++ b/test/regress_zlib.c
@@ -261,7 +261,7 @@ test_bufferevent_zlib(void *arg)
             = errorcb_invoked = 0;
 
 	if (evutil_socketpair(AF_UNIX, SOCK_STREAM, 0, pair) == -1) {
-		tt_fail_perror("socketpair");
+		tt_abort_perror("socketpair");
 	}
 
 	evutil_make_socket_nonblocking(pair[0]);
diff --git a/test/tinytest_macros.h b/test/tinytest_macros.h
--- a/test/tinytest_macros.h
+++ b/test/tinytest_macros.h
@@ -68,12 +68,16 @@
 	TT_STMT_END
 
 /* Fail and abort the current test for the reason in msg */
-#define tt_abort_msg(msg) TT_DIE((msg))
-#define tt_abort() tt_fail_msg("(Failed.)")
+#define tt_abort_printf(msg) TT_DIE(msg)
+#define tt_abort_perror(op) TT_DIE(("%s: %s [%d]",(op),strerror(errno), errno))
+#define tt_abort_msg(msg) TT_DIE(("%s", msg))
+#define tt_abort() TT_DIE(("%s", "(Failed.)"))
 
 /* Fail but do not abort the current test for the reason in msg. */
-#define tt_fail_msg(msg) TT_FAIL((msg))
-#define tt_fail() tt_fail_msg("(Failed.)")
+#define tt_fail_printf(msg) TT_FAIL(msg)
+#define tt_fail_perror(op) TT_FAIL(("%s: %s [%d]",(op),strerror(errno), errno))
+#define tt_fail_msg(msg) TT_FAIL(("%s", msg))
+#define tt_fail() TT_FAIL(("%s", "(Failed.)"))
 
 /* End the current test, and indicate we are skipping it. */
 #define tt_skip()                               \
@@ -135,10 +139,4 @@
 	tt_assert_test_type(a,b,#a" "#op" "#b,const char *,		\
 			    (strcmp(_val1,_val2) op 0),"<%s>")
 
-/** Fail and log the errno as with perror. */
-#define tt_fail_perror(op)					\
-	TT_STMT_BEGIN						\
-	TT_DIE(("%s: %s [%d]",(op),strerror(errno),errno));	\
-	TT_STMT_END
-
 #endif