From 5c4663772c8f7258a5ec5f329d2c6f4a60b1153a Mon Sep 17 00:00:00 2001 From: Anatoliy Bilenko Date: Tue, 27 Aug 2024 10:20:56 +0000 Subject: [PATCH 01/35] Added failure hook to discover https://github.com/canonical/dqlite/actions/runs/10565558010/job/29270343969 --- .github/workflows/build-and-test.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index d79b6a27f..665f85d55 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -50,6 +50,13 @@ jobs: run: | make check || (cat ./test-suite.log && false) + - name: OnTestFailure + if: ${{ failure() }} + uses: actions/upload-artifact@v3 + with: + name: test-suite.log + path: test-suite.log + - name: Coverage env: CC: ${{ matrix.compiler }} From 27eeeaf23622c138d4f1121c6ceb1116375c01ca Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Tue, 27 Aug 2024 14:55:23 -0400 Subject: [PATCH 02/35] Make sure test output is captured Signed-off-by: Cole Miller --- .github/workflows/build-and-test.yml | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 665f85d55..31da63589 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -48,14 +48,20 @@ jobs: CC: ${{ matrix.compiler }} LIBDQLITE_TRACE: 1 run: | - make check || (cat ./test-suite.log && false) + for bin in unit-test integration-test \ + raft-core-fuzzy-test raft-core-integration-test \ + raft-core-unit-test raft-uv-integration-test \ + raft-uv-unit-test + do ./$bin || touch any-failed + done 2>&1 | tee -a test-suite.log + test '!' -e any-failed - name: OnTestFailure if: ${{ failure() }} uses: actions/upload-artifact@v3 with: - name: test-suite.log - path: test-suite.log + name: test-suite.log + path: test-suite.log - name: Coverage env: From f1dc507c7c14967f1d5b5ba9071411d8195e58bc Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Wed, 28 Aug 2024 08:55:38 -0400 Subject: [PATCH 03/35] Revert "Merge pull request #687 from cole-miller/exec-sql-suspend-v2" This reverts commit 296fab962150985209c5a18dbad798e0612a33e4, reversing changes made to 6035a353ceb92226beccffbbd85f6aa3d1856a91. --- src/conn.c | 64 +++++----------------------------- src/conn.h | 5 --- src/gateway.c | 33 +++--------------- src/gateway.h | 24 +------------ src/leader.c | 5 --- src/leader.h | 28 +++++++-------- test/integration/test_client.c | 26 -------------- test/unit/test_concurrency.c | 15 ++------ test/unit/test_gateway.c | 15 ++------ 9 files changed, 31 insertions(+), 184 deletions(-) diff --git a/src/conn.c b/src/conn.c index 1bcdbcaff..f6adfa55d 100644 --- a/src/conn.c +++ b/src/conn.c @@ -100,27 +100,16 @@ static void gateway_handle_cb(struct handle *req, conn__stop(c); } -static void conn_defer_close_cb(uv_handle_t *t) +static void closeCb(struct transport *transport) { - struct conn *c = t->data; + struct conn *c = transport->data; + buffer__close(&c->write); + buffer__close(&c->read); if (c->close_cb != NULL) { c->close_cb(c); } } -static void conn_fini(struct conn *c) -{ - buffer__close(&c->write); - buffer__close(&c->read); - c->defer.data = c; - uv_close((uv_handle_t *)&c->defer, conn_defer_close_cb); -} - -/** - * This is called when raft wants to take over the connection to use for - * node-to-node communication. It consumes the conn and gives ownership of the - * underlying stream to raft. - */ static void raft_connect(struct conn *c) { struct cursor *cursor = &c->handle.cursor; @@ -135,14 +124,10 @@ static void raft_connect(struct conn *c) } raftProxyAccept(c->uv_transport, request.id, request.address, c->transport.stream); - /* Run the closing sequence to release resources held by the conn, - * since raft doesn't use the conn object or its buffers to handle - * incoming messages. However, don't close c->transport, because its - * uv_stream_t has been stolen by raft. */ - PRE(!c->closed); + /* Close the connection without actually closing the transport, since + * the stream will be used by raft */ c->closed = true; - gateway__close(&c->gateway); - conn_fini(c); + closeCb(&c->transport); } static void read_request_cb(struct transport *transport, int status) @@ -304,28 +289,6 @@ static int read_protocol(struct conn *c) return 0; } -static void conn_defer_cb(uv_timer_t *defer) -{ - struct conn *c = defer->data; - void (*cb)(void *) = c->defer_cb; - void *arg = c->defer_arg; - c->defer_cb = NULL; - c->defer_arg = NULL; - PRE(cb != NULL); - cb(arg); -} - -static void conn_defer(void (*cb)(void *arg), void *arg, void *data) -{ - struct conn *c = data; - PRE(c->defer_cb == NULL); - PRE(c->defer_arg == NULL); - c->defer_cb = cb; - c->defer_arg = arg; - c->defer.data = c; - uv_timer_start(&c->defer, conn_defer_cb, 0, 0); -} - int conn__start(struct conn *c, struct config *config, struct uv_loop_s *loop, @@ -348,10 +311,7 @@ int conn__start(struct conn *c, c->transport.data = c; c->uv_transport = uv_transport; c->close_cb = close_cb; - uv_timer_init(loop, &c->defer); - c->defer_cb = NULL; - c->defer_arg = NULL; - gateway__init(&c->gateway, config, registry, raft, seed, conn_defer, c); + gateway__init(&c->gateway, config, registry, raft, seed); rv = buffer__init(&c->read); if (rv != 0) { goto err_after_transport_init; @@ -379,12 +339,6 @@ int conn__start(struct conn *c, return rv; } -static void conn_transport_close_cb(struct transport *transport) -{ - struct conn *c = transport->data; - conn_fini(c); -} - void conn__stop(struct conn *c) { tracef("conn stop"); @@ -393,5 +347,5 @@ void conn__stop(struct conn *c) } c->closed = true; gateway__close(&c->gateway); - transport__close(&c->transport, conn_transport_close_cb); + transport__close(&c->transport, closeCb); } diff --git a/src/conn.h b/src/conn.h index b6b64bfab..0ae2b1299 100644 --- a/src/conn.h +++ b/src/conn.h @@ -33,11 +33,6 @@ struct conn struct message request; /* Request message meta data */ struct message response; /* Response message meta data */ struct handle handle; - /* Callback that the gateway has asked to be invoked on the next loop - * iteration, and its argument. */ - void (*defer_cb)(void *); - void *defer_arg; - uv_timer_t defer; bool closed; queue queue; }; diff --git a/src/gateway.c b/src/gateway.c index 082b1bacd..8cfe4fac1 100644 --- a/src/gateway.c +++ b/src/gateway.c @@ -18,9 +18,7 @@ void gateway__init(struct gateway *g, struct config *config, struct registry *registry, struct raft *raft, - struct id_state seed, - defer_impl defer, - void *defer_data) + struct id_state seed) { tracef("gateway init"); g->config = config; @@ -36,8 +34,6 @@ void gateway__init(struct gateway *g, g->protocol = DQLITE_PROTOCOL_VERSION; g->client_id = 0; g->random_state = seed; - g->defer = defer; - g->defer_data = defer_data; } /* FIXME: This function becomes unsound when using the new thread pool, since @@ -102,7 +98,6 @@ void gateway__leader_close(struct gateway *g, int reason) void gateway__close(struct gateway *g) { tracef("gateway close"); - if (g->leader == NULL) { stmt__registry_close(&g->stmts); return; @@ -725,14 +720,6 @@ static void handle_exec_sql_next(struct gateway *g, struct handle *req, bool done); -static void handle_exec_sql_next_defer_cb(void *arg) -{ - struct gateway *g = arg; - PRE(g != NULL && g->req != NULL); - PRE(g->req->type == DQLITE_REQUEST_EXEC_SQL); - handle_exec_sql_next(g, g->req, true); -} - static void handle_exec_sql_cb(struct exec *exec, int status) { tracef("handle exec sql cb status %d", status); @@ -742,24 +729,12 @@ static void handle_exec_sql_cb(struct exec *exec, int status) req->exec_count += 1; sqlite3_finalize(exec->stmt); - if (status != SQLITE_DONE) { + if (status == SQLITE_DONE) { + handle_exec_sql_next(g, req, true); + } else { assert(g->leader != NULL); failure(req, status, error_message(g->leader->conn, status)); g->req = NULL; - return; - } - - /* It would be valid to always invoke handle_exec_sql_next directly - * here. But that can lead to bounded recursion when we have several - * `;`-separated statements in a row that do not generate rows. To make - * sure the stack depth stays under control, we have the event loop - * loop invoke handle_exec_sql_next itself on the next iteration, but - * only if there is a prior call to handle_exec_sql_next above us on - * the stack. */ - if (exec->async) { - handle_exec_sql_next(g, req, true); - } else { - g->defer(handle_exec_sql_next_defer_cb, g, g->defer_data); } } diff --git a/src/gateway.h b/src/gateway.h index d9c210da0..2a5805084 100644 --- a/src/gateway.h +++ b/src/gateway.h @@ -19,21 +19,6 @@ struct handle; -/** - * Interface for requesting that a callback be invoked "later". - * - * When running on a libuv event loop, "later" means "on the next loop - * iteration". In the unit tests, where no loop is available, "later" means - * "right now". - * - * The gateway uses this interface to prevent recursion in the handler for - * EXEC_SQL requests. - * - * `arg` is provided by the gateway to be passed to `cb`, while `data` is - * the userdata passed by the owner of the gateway into gateway__init. - */ -typedef void (*defer_impl)(void (*cb)(void *arg), void *arg, void *data); - /** * Handle requests from a single connected client and forward them to * SQLite. @@ -50,20 +35,13 @@ struct gateway { uint64_t protocol; /* Protocol format version */ uint64_t client_id; struct id_state random_state; /* For generating IDs */ - defer_impl defer; - void *defer_data; }; -/** - * Initialize a gateway. - */ void gateway__init(struct gateway *g, struct config *config, struct registry *registry, struct raft *raft, - struct id_state seed, - defer_impl defer, - void *defer_data); + struct id_state seed); void gateway__close(struct gateway *g); diff --git a/src/leader.c b/src/leader.c index 0b4a4f23e..61ef91639 100644 --- a/src/leader.c +++ b/src/leader.c @@ -295,7 +295,6 @@ static void leaderApplyFramesCb(struct raft_apply *req, finish: l->inflight = NULL; l->db->tx_id = 0; - l->exec->async = true; leaderExecDone(l->exec); } @@ -440,8 +439,6 @@ static void execBarrierCb(struct barrier *barrier, int status) struct exec *req = barrier->data; struct leader *l = req->leader; - req->async = req->barrier.async; - if (status != 0) { l->exec->status = status; leaderExecDone(l->exec); @@ -508,7 +505,6 @@ static void raftBarrierCb(struct raft_barrier *req, int status) return; } barrier->cb = NULL; - barrier->async = true; cb(barrier, rv); } @@ -518,7 +514,6 @@ int leader__barrier(struct leader *l, struct barrier *barrier, barrier_cb cb) int rv; if (!needsBarrier(l)) { tracef("not needed"); - barrier->async = false; cb(barrier, 0); return 0; } diff --git a/src/leader.h b/src/leader.h index 21a6dbbee..9d022d3e9 100644 --- a/src/leader.h +++ b/src/leader.h @@ -49,9 +49,6 @@ struct barrier { void *data; struct leader *leader; struct raft_barrier req; - /* Whether we yielded control to the event loop at least once while - * handling the request. */ - bool async; barrier_cb cb; }; @@ -66,11 +63,8 @@ struct exec { uint64_t id; int status; queue queue; - pool_work_t work; - /* Whether we yielded control to the event loop at least once while - * handling the request. */ - bool async; exec_cb cb; + pool_work_t work; }; /** @@ -87,10 +81,17 @@ void leader__close(struct leader *l); /** * Submit a request to step a SQLite statement. * - * The callback may be invoked synchronously (i.e. while leader__exec is still - * on the stack) or asynchronously after suspending to wait for a raft request - * to finish. The callback can read the `async` field of the exec object to - * determine how it's being invoked. + * The request will be dispatched to the leader loop coroutine, which will be + * resumed and will invoke sqlite_step(). If the statement triggers the + * replication hooks and one or more new Raft log entries need to be appended, + * then the loop coroutine will be paused and control will be transferred back + * to the main coroutine. In this state the leader loop coroutine call stack + * will be "blocked" on the xFrames() replication hook call triggered by the top + * sqlite_step() call. The leader loop coroutine will be resumed once the Raft + * append request completes (either successfully or not) and at that point the + * stack will rewind back to the @sqlite_step() call, returning to the leader + * loop which will then have completed the request and transfer control back to + * the main coroutine, pausing until the next request. */ int leader__exec(struct leader *l, struct exec *req, @@ -102,10 +103,7 @@ int leader__exec(struct leader *l, * Submit a raft barrier request if there is no transaction in progress in the * underlying database and the FSM is behind the last log index. * - * If a barrier is needed, the callback is invoked asychronously after the - * raft barrier request has completed. Otherwise, the callback is invoked - * synchronously. The callback can read the `async` field of the barrier object - * to determine how it's being invoked. + * Otherwise, just invoke the given @cb immediately. */ int leader__barrier(struct leader *l, struct barrier *barrier, barrier_cb cb); diff --git a/test/integration/test_client.c b/test/integration/test_client.c index c2a1cfec2..98fccd428 100644 --- a/test/integration/test_client.c +++ b/test/integration/test_client.c @@ -145,29 +145,3 @@ TEST(client, querySql, setUp, tearDown, 0, client_params) return MUNIT_OK; } - -/* Stress test of an EXEC_SQL with many ';'-separated statements. */ -TEST(client, semicolons, setUp, tearDown, 0, NULL) -{ - struct fixture *f = data; - (void)params; - - static const char trivial_stmt[] = "CREATE TABLE IF NOT EXISTS foo (n INT);"; - - size_t n = 1000; - size_t unit = sizeof(trivial_stmt) - 1; - char *sql = munit_malloc(n * unit); - char *p = sql; - for (size_t i = 0; i < n; i++) { - memcpy(p, trivial_stmt, unit); - p += unit; - } - sql[n * unit - 1] = '\0'; - - uint64_t last_insert_id; - uint64_t rows_affected; - EXEC_SQL(sql, &last_insert_id, &rows_affected); - - free(sql); - return MUNIT_OK; -} diff --git a/test/unit/test_concurrency.c b/test/unit/test_concurrency.c index c59dd65fd..6008efacc 100644 --- a/test/unit/test_concurrency.c +++ b/test/unit/test_concurrency.c @@ -33,12 +33,6 @@ struct connection { struct context context; }; -static void defer_run_now(void (*cb)(void *arg), void *arg, void *data) -{ - (void)data; - cb(arg); -} - #define FIXTURE \ FIXTURE_CLUSTER; \ struct connection connections[N_GATEWAYS] @@ -55,13 +49,8 @@ static void defer_run_now(void (*cb)(void *arg), void *arg, void *data) struct request_open open; \ struct response_db db; \ struct id_state seed = { { 1 } }; \ - gateway__init(&c->gateway, \ - CLUSTER_CONFIG(0), \ - CLUSTER_REGISTRY(0), \ - CLUSTER_RAFT(0), \ - seed, \ - defer_run_now, \ - NULL); \ + gateway__init(&c->gateway, CLUSTER_CONFIG(0), \ + CLUSTER_REGISTRY(0), CLUSTER_RAFT(0), seed); \ c->handle.data = &c->context; \ rc = buffer__init(&c->request); \ munit_assert_int(rc, ==, 0); \ diff --git a/test/unit/test_gateway.c b/test/unit/test_gateway.c index 0e8122859..d0e1fb7c6 100644 --- a/test/unit/test_gateway.c +++ b/test/unit/test_gateway.c @@ -35,12 +35,6 @@ struct connection { struct context context; }; -static void defer_run_now(void (*cb)(void *arg), void *arg, void *data) -{ - (void)data; - cb(arg); -} - #define FIXTURE \ FIXTURE_CLUSTER; \ struct connection connections[N_SERVERS]; \ @@ -61,13 +55,8 @@ static void defer_run_now(void (*cb)(void *arg), void *arg, void *data) struct id_state seed = { { 1 } }; \ config = CLUSTER_CONFIG(i); \ config->page_size = 512; \ - gateway__init(&c->gateway, \ - config, \ - CLUSTER_REGISTRY(i), \ - CLUSTER_RAFT(i), \ - seed, \ - defer_run_now, \ - NULL); \ + gateway__init(&c->gateway, config, CLUSTER_REGISTRY(i), \ + CLUSTER_RAFT(i), seed); \ c->handle.data = &c->context; \ rc = buffer__init(&c->buf1); \ munit_assert_int(rc, ==, 0); \ From 87e8e6bf62d44edbbf66394d203ba16daf4c0f7f Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Tue, 20 Aug 2024 19:39:08 -0400 Subject: [PATCH 04/35] test: Split raft uv tests that rely on mocking getaddrinfo Signed-off-by: Cole Miller --- .github/workflows/build-and-test.yml | 1 + Makefile.am | 60 ++-- test/raft/integration/main_addrinfo.c | 3 + test/raft/integration/test_uv_tcp_connect.c | 53 ---- .../test_uv_tcp_connect_addrinfo.c | 231 +++++++++++++++ test/raft/integration/test_uv_tcp_listen.c | 80 ------ .../integration/test_uv_tcp_listen_addrinfo.c | 268 ++++++++++++++++++ 7 files changed, 539 insertions(+), 157 deletions(-) create mode 100644 test/raft/integration/main_addrinfo.c create mode 100644 test/raft/integration/test_uv_tcp_connect_addrinfo.c create mode 100644 test/raft/integration/test_uv_tcp_listen_addrinfo.c diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 31da63589..23ac612ee 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -41,6 +41,7 @@ jobs: raft-core-integration-test \ raft-core-unit-test \ raft-uv-integration-test \ + raft-addrinfo-integration-test \ raft-uv-unit-test - name: Test diff --git a/Makefile.am b/Makefile.am index bb780ae1c..9c1285acf 100644 --- a/Makefile.am +++ b/Makefile.am @@ -46,8 +46,6 @@ basic_dqlite_sources = \ src/lib/addr.c \ src/lib/buffer.c \ src/lib/fs.c \ - src/lib/sm.c \ - src/lib/threadpool.c \ src/lib/transport.c \ src/logger.c \ src/message.c \ @@ -60,7 +58,6 @@ basic_dqlite_sources = \ src/roles.c \ src/server.c \ src/stmt.c \ - src/tracing.c \ src/transport.c \ src/translate.c \ src/tuple.c \ @@ -71,6 +68,7 @@ lib_LTLIBRARIES = libdqlite.la libdqlite_la_CFLAGS = $(AM_CFLAGS) -fvisibility=hidden -DRAFT_API='' libdqlite_la_LDFLAGS = $(AM_LDFLAGS) -version-info 0:1:0 libdqlite_la_SOURCES = $(basic_dqlite_sources) +libdqlite_la_LIBADD = libcommon.la if BUILD_RAFT_ENABLED libraft_la_SOURCES = \ @@ -128,9 +126,10 @@ libraft_la_SOURCES = \ libdqlite_la_SOURCES += $(libraft_la_SOURCES) endif # BUILD_RAFT_ENABLED +# When adding a new test binary, make sure to update CI. check_PROGRAMS = unit-test integration-test -check_LTLIBRARIES = libtest.la +check_LTLIBRARIES = libtest.la libcommon.la libtest_la_CFLAGS = $(AM_CFLAGS) -DMUNIT_TEST_NAME_LEN=60 -Wno-unknown-warning-option -Wno-unused-result -Wno-conversion -Wno-uninitialized -Wno-maybe-uninitialized -Wno-strict-prototypes -Wno-old-style-definition libtest_la_SOURCES = \ @@ -145,6 +144,16 @@ libtest_la_SOURCES = \ test/lib/sqlite.c \ test/lib/uv.c +# libcommon is the destination for code that is used by both the raft +# half and the dqlite half of the codebase, but isn't part of the raft +# API surface. It's also used to make these bits of code accessible to +# the unit tests without compiling the same few files repeatedly. +libcommon_la_CFLAGS = $(AM_CFLAGS) +libcommon_la_SOURCES = \ + src/tracing.c \ + src/lib/sm.c \ + src/lib/threadpool.c + unit_test_SOURCES = $(basic_dqlite_sources) unit_test_SOURCES += \ test/test_error.c \ @@ -172,7 +181,7 @@ unit_test_SOURCES += \ test/unit/main.c unit_test_CFLAGS = $(AM_CFLAGS) -Wno-unknown-warning-option -Wno-uninitialized -Wno-maybe-uninitialized -Wno-float-equal -Wno-conversion unit_test_LDFLAGS = $(AM_LDFLAGS) -unit_test_LDADD = libtest.la +unit_test_LDADD = libcommon.la libtest.la if BUILD_RAFT_ENABLED unit_test_LDADD += libraft.la @@ -195,15 +204,16 @@ integration_test_LDADD = libtest.la libdqlite.la if BUILD_RAFT_ENABLED check_LTLIBRARIES += libraft.la +# When adding a new test binary, make sure to update CI. check_PROGRAMS += \ raft-core-unit-test \ raft-core-integration-test \ raft-uv-unit-test \ raft-uv-integration-test \ + raft-addrinfo-integration-test \ raft-core-fuzzy-test libtest_la_SOURCES += \ - test/raft/lib/addrinfo.c \ test/raft/lib/fault.c \ test/raft/lib/fsm.c \ test/raft/lib/heap.c \ @@ -217,12 +227,10 @@ libtest_la_SOURCES += \ libraft_la_CFLAGS = $(AM_CFLAGS) libraft_la_LDFLAGS = $(UV_LIBS) +libraft_la_LIBADD = libcommon.la raft_core_unit_test_SOURCES = \ $(libraft_la_SOURCES) \ - src/lib/sm.c \ - src/lib/threadpool.c \ - src/tracing.c \ test/raft/unit/main_core.c \ test/raft/unit/test_byte.c \ test/raft/unit/test_compress.c \ @@ -234,12 +242,9 @@ raft_core_unit_test_SOURCES = \ test/raft/unit/test_snapshot.c raft_core_unit_test_CFLAGS = $(AM_CFLAGS) -Wno-conversion -raft_core_unit_test_LDADD = libtest.la +raft_core_unit_test_LDADD = libcommon.la libtest.la raft_core_integration_test_SOURCES = \ - src/tracing.c \ - src/lib/sm.c \ - src/lib/threadpool.c \ test/raft/integration/main_core.c \ test/raft/integration/test_apply.c \ test/raft/integration/test_assign.c \ @@ -264,9 +269,6 @@ raft_core_integration_test_LDFLAGS = -no-install raft_core_integration_test_LDADD = libtest.la libraft.la raft_core_fuzzy_test_SOURCES = \ - src/lib/sm.c \ - src/lib/threadpool.c \ - src/tracing.c \ test/raft/fuzzy/main_core.c \ test/raft/fuzzy/test_election.c \ test/raft/fuzzy/test_liveness.c \ @@ -277,7 +279,6 @@ raft_core_fuzzy_test_LDFLAGS = -no-install raft_core_fuzzy_test_LDADD = libtest.la libraft.la raft_uv_unit_test_SOURCES = \ - src/tracing.c \ src/raft/err.c \ src/raft/heap.c \ src/raft/syscall.c \ @@ -289,16 +290,12 @@ raft_uv_unit_test_SOURCES = \ test/raft/unit/test_uv_os.c \ test/raft/unit/test_uv_writer.c raft_uv_unit_test_CFLAGS = $(AM_CFLAGS) -Wno-conversion -raft_uv_unit_test_LDADD = libtest.la $(UV_LIBS) +raft_uv_unit_test_LDADD = libcommon.la libtest.la $(UV_LIBS) # The integration/uv test is not linked to libraft, but built # directly against the libraft sources in order to test some # non-visible, non-API functions. raft_uv_integration_test_SOURCES = \ - $(libraft_la_SOURCES) \ - src/tracing.c \ - src/lib/sm.c \ - src/lib/threadpool.c \ test/raft/integration/main_uv.c \ test/raft/integration/test_uv_init.c \ test/raft/integration/test_uv_append.c \ @@ -316,7 +313,22 @@ raft_uv_integration_test_SOURCES = \ test/raft/integration/test_uv_work.c raft_uv_integration_test_CFLAGS = $(AM_CFLAGS) -Wno-type-limits -Wno-conversion raft_uv_integration_test_LDFLAGS = -no-install -raft_uv_integration_test_LDADD = libtest.la $(UV_LIBS) +raft_uv_integration_test_LDADD = libtest.la libraft.la $(UV_LIBS) + +# Some of the uv integration tests are broken out into a separate +# binary because they use a trick to override libc's getaddrinfo +# with a mock implementation. When libc is statically linked (a use-case +# that we care about), this trick doesn't work, and prevents any +# test in the same binary that calls getaddrinfo from working +# correctly. +raft_addrinfo_integration_test_SOURCES = \ + test/raft/lib/addrinfo.c \ + test/raft/integration/test_uv_tcp_connect_addrinfo.c \ + test/raft/integration/test_uv_tcp_listen_addrinfo.c \ + test/raft/integration/main_addrinfo.c +raft_addrinfo_integration_test_CFLAGS = $(AM_CFLAGS) -Wno-type-limits -Wno-conversion +raft_addrinfo_integration_test_LDFLAGS = -no-install +raft_addrinfo_integration_test_LDADD = libtest.la libraft.la $(UV_LIBS) if LZ4_AVAILABLE libdqlite_la_CFLAGS += -DLZ4_AVAILABLE $(LZ4_CFLAGS) @@ -344,7 +356,7 @@ libsqlite3_la_SOURCES = sqlite3.c libsqlite3_la_CFLAGS = -g3 unit_test_LDADD += libsqlite3.la -libdqlite_la_LIBADD = libsqlite3.la +libdqlite_la_LIBADD += libsqlite3.la else AM_LDFLAGS += $(SQLITE_LIBS) endif diff --git a/test/raft/integration/main_addrinfo.c b/test/raft/integration/main_addrinfo.c new file mode 100644 index 000000000..07476ef73 --- /dev/null +++ b/test/raft/integration/main_addrinfo.c @@ -0,0 +1,3 @@ +#include "../lib/runner.h" + +RUNNER("addrinfo") diff --git a/test/raft/integration/test_uv_tcp_connect.c b/test/raft/integration/test_uv_tcp_connect.c index 7efc68c60..b6395c6f0 100644 --- a/test/raft/integration/test_uv_tcp_connect.c +++ b/test/raft/integration/test_uv_tcp_connect.c @@ -1,6 +1,5 @@ #include "../../../src/raft.h" #include "../../../src/raft.h" -#include "../lib/addrinfo.h" #include "../lib/heap.h" #include "../lib/loop.h" #include "../lib/runner.h" @@ -126,7 +125,6 @@ static void *setUpDeps(const MunitParameter params[], { struct fixture *f = munit_malloc(sizeof *f); int rv; - SET_UP_ADDRINFO; SET_UP_HEAP; SETUP_LOOP; SETUP_TCP_SERVER; @@ -144,7 +142,6 @@ static void tearDownDeps(void *data) TEAR_DOWN_TCP_SERVER; TEAR_DOWN_LOOP; TEAR_DOWN_HEAP; - TEAR_DOWN_ADDRINFO; free(f); } @@ -191,29 +188,6 @@ TEST(tcp_connect, connectByName, setUp, tearDown, 0, NULL) return MUNIT_OK; } -/* Successfully connect to the peer by first IP */ -TEST(tcp_connect, firstIP, setUp, tearDown, 0, NULL) -{ - struct fixture *f = data; - const struct AddrinfoResult results[] = {{"127.0.0.1", TCP_SERVER_PORT}, - {"192.0.2.0", 6666}}; - AddrinfoInjectSetResponse(0, 2, results); - CONNECT(2, "any-host"); - return MUNIT_OK; -} - -/* Successfully connect to the peer by second IP */ -TEST(tcp_connect, secondIP, setUp, tearDown, 0, NULL) -{ - struct fixture *f = data; - const struct AddrinfoResult results[] = {{"127.0.0.1", .6666}, - {"127.0.0.1", TCP_SERVER_PORT}}; - - AddrinfoInjectSetResponse(0, 2, results); - CONNECT(2, "any-host"); - return MUNIT_OK; -} - /* The peer has shutdown */ TEST(tcp_connect, refused, setUp, tearDown, 0, NULL) { @@ -329,30 +303,3 @@ TEST(tcp_connect, closeDuringConnectAbort, setUp, tearDownDeps, 0, NULL) CLOSE_WAIT; return MUNIT_OK; } - -/* The transport gets closed right after the first connection attempt failed, - * while doing a second connection attempt. */ -TEST(tcp_connect, closeDuringSecondConnect, setUp, tearDownDeps, 0, NULL) -{ - struct fixture *f = data; - struct uv_check_s check; - int rv; - const struct AddrinfoResult results[] = {{"127.0.0.1", .6666}, - {"127.0.0.1", TCP_SERVER_PORT}}; - - AddrinfoInjectSetResponse(0, 2, results); - - /* Use a check handle in order to close the transport in the same loop - * iteration where the second connection attempt occurs. */ - rv = uv_check_init(&f->loop, &check); - munit_assert_int(rv, ==, 0); - check.data = f; - CONNECT_REQ(2, "any-host", 0, RAFT_CANCELED); - /* Successfull DNS lookup will initiate async connect */ - LOOP_RUN(1); - uv_check_start(&check, checkCb); - LOOP_RUN(1); - LOOP_RUN_UNTIL(&_result.done); - CLOSE_WAIT; - return MUNIT_OK; -} diff --git a/test/raft/integration/test_uv_tcp_connect_addrinfo.c b/test/raft/integration/test_uv_tcp_connect_addrinfo.c new file mode 100644 index 000000000..969b24ead --- /dev/null +++ b/test/raft/integration/test_uv_tcp_connect_addrinfo.c @@ -0,0 +1,231 @@ +#include "../../../src/raft.h" +#include "../../../src/raft.h" +#include "../lib/addrinfo.h" +#include "../lib/heap.h" +#include "../lib/loop.h" +#include "../lib/runner.h" +#include "../lib/tcp.h" + +/****************************************************************************** + * + * Fixture with a TCP-based raft_uv_transport. + * + *****************************************************************************/ + +struct fixture +{ + FIXTURE_HEAP; + FIXTURE_LOOP; + FIXTURE_TCP_SERVER; + struct raft_uv_transport transport; + bool closed; +}; + +/****************************************************************************** + * + * Helper macros + * + *****************************************************************************/ + +struct result +{ + int status; + bool done; +}; + +static void closeCb(struct raft_uv_transport *transport) +{ + struct fixture *f = transport->data; + f->closed = true; +} + +static void connectCbAssertResult(struct raft_uv_connect *req, + struct uv_stream_s *stream, + int status) +{ + struct result *result = req->data; + munit_assert_int(status, ==, result->status); + if (status == 0) { + uv_close((struct uv_handle_s *)stream, (uv_close_cb)raft_free); + } + result->done = true; +} + +#define INIT \ + do { \ + int _rv; \ + _rv = f->transport.init(&f->transport, 1, "127.0.0.1:9000"); \ + munit_assert_int(_rv, ==, 0); \ + f->transport.data = f; \ + f->closed = false; \ + } while (0) + +#define CLOSE_SUBMIT \ + munit_assert_false(f->closed); \ + f->transport.close(&f->transport, closeCb); + +#define CLOSE_WAIT LOOP_RUN_UNTIL(&f->closed) +#define CLOSE \ + CLOSE_SUBMIT; \ + CLOSE_WAIT + +#define CONNECT_REQ(ID, ADDRESS, RV, STATUS) \ + struct raft_uv_connect _req; \ + struct result _result = {STATUS, false}; \ + int _rv; \ + _req.data = &_result; \ + _rv = f->transport.connect(&f->transport, &_req, ID, ADDRESS, \ + connectCbAssertResult); \ + munit_assert_int(_rv, ==, RV) + +/* Try to submit a connect request and assert that the given error code and + * message are returned. */ +#define CONNECT_ERROR(ID, ADDRESS, RV, ERRMSG) \ + { \ + CONNECT_REQ(ID, ADDRESS, RV /* rv */, 0 /* status */); \ + munit_assert_string_equal(f->transport.errmsg, ERRMSG); \ + } + +/* Submit a connect request with the given parameters and wait for the operation + * to successfully complete. */ +#define CONNECT(ID, ADDRESS) \ + { \ + CONNECT_REQ(ID, ADDRESS, 0 /* rv */, 0 /* status */); \ + LOOP_RUN_UNTIL(&_result.done); \ + } + +/* Submit a connect request with the given parameters and wait for the operation + * to fail with the given code and message. */ +#define CONNECT_FAILURE(ID, ADDRESS, STATUS, ERRMSG) \ + { \ + CONNECT_REQ(ID, ADDRESS, 0 /* rv */, STATUS); \ + LOOP_RUN_UNTIL(&_result.done); \ + munit_assert_string_equal(f->transport.errmsg, ERRMSG); \ + } + +/* Submit a connect request with the given parameters, close the transport after + * N loop iterations and assert that the request got canceled. */ +#define CONNECT_CLOSE(ID, ADDRESS, N) \ + { \ + CONNECT_REQ(ID, ADDRESS, 0 /* rv */, RAFT_CANCELED); \ + LOOP_RUN(N); \ + CLOSE_SUBMIT; \ + munit_assert_false(_result.done); \ + LOOP_RUN_UNTIL(&_result.done); \ + CLOSE_WAIT; \ + } + +/****************************************************************************** + * + * Set up and tear down. + * + *****************************************************************************/ + +static void *setUpDeps(const MunitParameter params[], + MUNIT_UNUSED void *user_data) +{ + struct fixture *f = munit_malloc(sizeof *f); + int rv; + SET_UP_ADDRINFO; + SET_UP_HEAP; + SETUP_LOOP; + SETUP_TCP_SERVER; + f->transport.version = 1; + rv = raft_uv_tcp_init(&f->transport, &f->loop); + munit_assert_int(rv, ==, 0); + return f; +} + +static void tearDownDeps(void *data) +{ + struct fixture *f = data; + LOOP_STOP; + raft_uv_tcp_close(&f->transport); + TEAR_DOWN_TCP_SERVER; + TEAR_DOWN_LOOP; + TEAR_DOWN_HEAP; + TEAR_DOWN_ADDRINFO; + free(f); +} + +static void *setUp(const MunitParameter params[], void *user_data) +{ + struct fixture *f = setUpDeps(params, user_data); + INIT; + return f; +} + +static void tearDown(void *data) +{ + struct fixture *f = data; + CLOSE; + tearDownDeps(f); +} + +/****************************************************************************** + * + * raft_uv_transport->connect() + * + *****************************************************************************/ + +#define BOGUS_ADDRESS "127.0.0.1:6666" +#define INVALID_ADDRESS "500.0.0.1:6666" + +SUITE(tcp_connect) + +/* Successfully connect to the peer by first IP */ +TEST(tcp_connect, firstIP, setUp, tearDown, 0, NULL) +{ + struct fixture *f = data; + const struct AddrinfoResult results[] = {{"127.0.0.1", TCP_SERVER_PORT}, + {"192.0.2.0", 6666}}; + AddrinfoInjectSetResponse(0, 2, results); + CONNECT(2, "any-host"); + return MUNIT_OK; +} + +/* Successfully connect to the peer by second IP */ +TEST(tcp_connect, secondIP, setUp, tearDown, 0, NULL) +{ + struct fixture *f = data; + const struct AddrinfoResult results[] = {{"127.0.0.1", .6666}, + {"127.0.0.1", TCP_SERVER_PORT}}; + + AddrinfoInjectSetResponse(0, 2, results); + CONNECT(2, "any-host"); + return MUNIT_OK; +} + +static void checkCb(struct uv_check_s *check) +{ + struct fixture *f = check->data; + CLOSE_SUBMIT; + uv_close((struct uv_handle_s *)check, NULL); +} + +/* The transport gets closed right after the first connection attempt failed, + * while doing a second connection attempt. */ +TEST(tcp_connect, closeDuringSecondConnect, setUp, tearDownDeps, 0, NULL) +{ + struct fixture *f = data; + struct uv_check_s check; + int rv; + const struct AddrinfoResult results[] = {{"127.0.0.1", .6666}, + {"127.0.0.1", TCP_SERVER_PORT}}; + + AddrinfoInjectSetResponse(0, 2, results); + + /* Use a check handle in order to close the transport in the same loop + * iteration where the second connection attempt occurs. */ + rv = uv_check_init(&f->loop, &check); + munit_assert_int(rv, ==, 0); + check.data = f; + CONNECT_REQ(2, "any-host", 0, RAFT_CANCELED); + /* Successfull DNS lookup will initiate async connect */ + LOOP_RUN(1); + uv_check_start(&check, checkCb); + LOOP_RUN(1); + LOOP_RUN_UNTIL(&_result.done); + CLOSE_WAIT; + return MUNIT_OK; +} diff --git a/test/raft/integration/test_uv_tcp_listen.c b/test/raft/integration/test_uv_tcp_listen.c index b239cfa87..747add3a6 100644 --- a/test/raft/integration/test_uv_tcp_listen.c +++ b/test/raft/integration/test_uv_tcp_listen.c @@ -1,6 +1,5 @@ #include "../../../src/raft.h" #include "../../../src/raft/byte.h" -#include "../lib/addrinfo.h" #include "../lib/heap.h" #include "../lib/loop.h" #include "../lib/runner.h" @@ -95,7 +94,6 @@ static void *setUpDeps(const MunitParameter params[], MUNIT_UNUSED void *user_data) { struct fixture *f = munit_malloc(sizeof *f); - SET_UP_ADDRINFO; SET_UP_HEAP; SETUP_LOOP; SETUP_TCP; @@ -108,7 +106,6 @@ static void tearDownDeps(void *data) TEAR_DOWN_TCP; TEAR_DOWN_LOOP; TEAR_DOWN_HEAP; - TEAR_DOWN_ADDRINFO; free(f); } @@ -235,83 +232,6 @@ TEST(tcp_listen, invalidAddress, setUp, tearDown, 0, invalidTcpListenParams) return MUNIT_OK; } -/* Check success with addrinfo resolve to mutiple IP and first one is used to - * connect */ -TEST(tcp_listen, firstOfTwo, setUp, tearDown, 0, NULL) -{ - const struct AddrinfoResult results[] = {{"127.0.0.1", 9000}, - {"127.0.0.2", 9000}}; - struct fixture *f = data; - AddrinfoInjectSetResponse(0, 2, results); - LISTEN(0); - PEER_CONNECT; - PEER_HANDSHAKE; - ACCEPT; - return MUNIT_OK; -} - -/* Check success with addrinfo resolve to mutiple IP and second one is used to - * connect */ -TEST(tcp_listen, secondOfTwo, setUp, tearDown, 0, NULL) -{ - const struct AddrinfoResult results[] = {{"127.0.0.2", 9000}, - {"127.0.0.1", 9000}}; - struct fixture *f = data; - AddrinfoInjectSetResponse(0, 2, results); - - LISTEN(0); - PEER_CONNECT; - PEER_HANDSHAKE; - ACCEPT; - return MUNIT_OK; -} - -/* Simulate port already in use error by addrinfo response contain the same IP - * twice */ -TEST(tcp_listen, alreadyBound, setUp, tearDown, 0, NULL) -{ - /* We need to use the same endpoint three times as a simple duplicate will - * be skipped due to a glib strange behavior - * https://bugzilla.redhat.com/show_bug.cgi?id=496300 */ - const struct AddrinfoResult results[] = { - {"127.0.0.1", 9000}, {"127.0.0.1", 9000}, {"127.0.0.1", 9000}}; - struct fixture *f = data; - AddrinfoInjectSetResponse(0, 3, results); - LISTEN(RAFT_IOERR); - return MUNIT_OK; -} - -/* Error in bind first IP address */ -TEST(tcp_listen, cannotBindFirst, setUp, tearDown, 0, NULL) -{ - const struct AddrinfoResult results[] = {{"192.0.2.0", 9000}, - {"127.0.0.1", 9000}}; - struct fixture *f = data; - AddrinfoInjectSetResponse(0, 2, results); - LISTEN(RAFT_IOERR); - return MUNIT_OK; -} - -/* Error in bind of second IP address */ -TEST(tcp_listen, cannotBindSecond, setUp, tearDown, 0, NULL) -{ - const struct AddrinfoResult results[] = {{"127.0.0.1", 9000}, - {"192.0.2.0", 9000}}; - struct fixture *f = data; - AddrinfoInjectSetResponse(0, 2, results); - LISTEN(RAFT_IOERR); - return MUNIT_OK; -} - -/* Check error on general dns server failure */ -TEST(tcp_listen, resolveFailure, setUp, tearDown, 0, NULL) -{ - struct fixture *f = data; - AddrinfoInjectSetResponse(EAI_FAIL, 0, NULL); - LISTEN(RAFT_IOERR); - return MUNIT_OK; -} - /* The client sends us a bad protocol version */ TEST(tcp_listen, badProtocol, setUp, tearDown, 0, NULL) { diff --git a/test/raft/integration/test_uv_tcp_listen_addrinfo.c b/test/raft/integration/test_uv_tcp_listen_addrinfo.c new file mode 100644 index 000000000..dc7123362 --- /dev/null +++ b/test/raft/integration/test_uv_tcp_listen_addrinfo.c @@ -0,0 +1,268 @@ +#include "../../../src/raft.h" +#include "../../../src/raft/byte.h" +#include "../lib/addrinfo.h" +#include "../lib/heap.h" +#include "../lib/loop.h" +#include "../lib/runner.h" +#include "../lib/tcp.h" + +/****************************************************************************** + * + * Fixture with a TCP-based raft_uv_transport. + * + *****************************************************************************/ + +struct fixture +{ + FIXTURE_HEAP; + FIXTURE_LOOP; + FIXTURE_TCP; + struct raft_uv_transport transport; + bool accepted; + bool closed; + struct + { + uint8_t buf[sizeof(uint64_t) + /* Protocol version */ + sizeof(uint64_t) + /* Server ID */ + sizeof(uint64_t) + /* Length of address */ + sizeof(uint64_t) * 2 /* Address */]; + size_t offset; + } handshake; +}; + +/****************************************************************************** + * + * Helper macros + * + *****************************************************************************/ + +#define PEER_ID 2 +#define PEER_ADDRESS "127.0.0.1:666" + +static void closeCb(struct raft_uv_transport *transport) +{ + struct fixture *f = transport->data; + f->closed = true; +} + +static void acceptCb(struct raft_uv_transport *t, + raft_id id, + const char *address, + struct uv_stream_s *stream) +{ + struct fixture *f = t->data; + munit_assert_int(id, ==, PEER_ID); + munit_assert_string_equal(address, PEER_ADDRESS); + f->accepted = true; + uv_close((struct uv_handle_s *)stream, (uv_close_cb)raft_free); +} + +#define INIT \ + do { \ + int _rv; \ + f->transport.version = 1; \ + _rv = raft_uv_tcp_init(&f->transport, &f->loop); \ + munit_assert_int(_rv, ==, 0); \ + const char *bind_addr = munit_parameters_get(params, "bind-address"); \ + if (bind_addr && strlen(bind_addr)) { \ + _rv = raft_uv_tcp_set_bind_address(&f->transport, bind_addr); \ + munit_assert_int(_rv, ==, 0); \ + } \ + const char *address = munit_parameters_get(params, "address"); \ + if (!address) { \ + address = "127.0.0.1:9000"; \ + } \ + _rv = f->transport.init(&f->transport, 1, address); \ + munit_assert_int(_rv, ==, 0); \ + f->transport.data = f; \ + f->closed = false; \ + } while (0) + +#define CLOSE \ + do { \ + f->transport.close(&f->transport, closeCb); \ + LOOP_RUN_UNTIL(&f->closed); \ + raft_uv_tcp_close(&f->transport); \ + } while (0) + +/****************************************************************************** + * + * Set up and tear down. + * + *****************************************************************************/ + +static void *setUpDeps(const MunitParameter params[], + MUNIT_UNUSED void *user_data) +{ + struct fixture *f = munit_malloc(sizeof *f); + SET_UP_ADDRINFO; + SET_UP_HEAP; + SETUP_LOOP; + SETUP_TCP; + return f; +} + +static void tearDownDeps(void *data) +{ + struct fixture *f = data; + TEAR_DOWN_TCP; + TEAR_DOWN_LOOP; + TEAR_DOWN_HEAP; + TEAR_DOWN_ADDRINFO; + free(f); +} + +static void *setUp(const MunitParameter params[], void *user_data) +{ + struct fixture *f = setUpDeps(params, user_data); + void *cursor; + /* test_tcp_listen(&f->tcp); */ + INIT; + f->accepted = false; + f->handshake.offset = 0; + + cursor = f->handshake.buf; + bytePut64(&cursor, 1); + bytePut64(&cursor, PEER_ID); + bytePut64(&cursor, 16); + strcpy(cursor, PEER_ADDRESS); + + return f; +} + +static void tearDown(void *data) +{ + struct fixture *f = data; + CLOSE; + tearDownDeps(f); +} + +/****************************************************************************** + * + * Helper macros + * + *****************************************************************************/ + +#define LISTEN(EXPECTED_RV) \ + do { \ + int rv; \ + rv = f->transport.listen(&f->transport, acceptCb); \ + munit_assert_int(rv, ==, EXPECTED_RV); \ + } while (false) + +/* Connect to the listening socket of the transport, creating a new connection + * that is waiting to be accepted. */ +#define PEER_CONNECT TCP_CLIENT_CONNECT(9000) + +/* Make the peer close the connection. */ +#define PEER_CLOSE TCP_CLIENT_CLOSE + +/* Make the connected client send handshake data. */ +#define PEER_HANDSHAKE \ + do { \ + size_t n = sizeof f->handshake.buf; \ + TCP_CLIENT_SEND(f->handshake.buf, n); \ + } while (0) + +/* Make the connected client send partial handshake data: only N bytes will be + * sent, starting from the offset of the last call. */ +#define PEER_HANDSHAKE_PARTIAL(N) \ + do { \ + TCP_CLIENT_SEND(f->handshake.buf + f->handshake.offset, N); \ + } while (0) + +/* After a PEER_CONNECT() call, spin the event loop until the connected + * callback of the listening TCP handle gets called. */ +#define LOOP_RUN_UNTIL_CONNECTED LOOP_RUN(1); + +/* After a PEER_HANDSHAKE_PARTIAL() call, spin the event loop until the read + * callback gets called. */ +#define LOOP_RUN_UNTIL_READ LOOP_RUN(1); + +/* Spin the event loop until the accept callback gets eventually invoked. */ +#define ACCEPT LOOP_RUN_UNTIL(&f->accepted); + +/****************************************************************************** + * + * Success scenarios. + * + *****************************************************************************/ + +SUITE(tcp_listen) + +/* Check success with addrinfo resolve to mutiple IP and first one is used to + * connect */ +TEST(tcp_listen, firstOfTwo, setUp, tearDown, 0, NULL) +{ + const struct AddrinfoResult results[] = {{"127.0.0.1", 9000}, + {"127.0.0.2", 9000}}; + struct fixture *f = data; + AddrinfoInjectSetResponse(0, 2, results); + LISTEN(0); + PEER_CONNECT; + PEER_HANDSHAKE; + ACCEPT; + return MUNIT_OK; +} + +/* Check success with addrinfo resolve to mutiple IP and second one is used to + * connect */ +TEST(tcp_listen, secondOfTwo, setUp, tearDown, 0, NULL) +{ + const struct AddrinfoResult results[] = {{"127.0.0.2", 9000}, + {"127.0.0.1", 9000}}; + struct fixture *f = data; + AddrinfoInjectSetResponse(0, 2, results); + + LISTEN(0); + PEER_CONNECT; + PEER_HANDSHAKE; + ACCEPT; + return MUNIT_OK; +} + +/* Simulate port already in use error by addrinfo response contain the same IP + * twice */ +TEST(tcp_listen, alreadyBound, setUp, tearDown, 0, NULL) +{ + /* We need to use the same endpoint three times as a simple duplicate will + * be skipped due to a glib strange behavior + * https://bugzilla.redhat.com/show_bug.cgi?id=496300 */ + const struct AddrinfoResult results[] = { + {"127.0.0.1", 9000}, {"127.0.0.1", 9000}, {"127.0.0.1", 9000}}; + struct fixture *f = data; + AddrinfoInjectSetResponse(0, 3, results); + LISTEN(RAFT_IOERR); + return MUNIT_OK; +} + +/* Error in bind first IP address */ +TEST(tcp_listen, cannotBindFirst, setUp, tearDown, 0, NULL) +{ + const struct AddrinfoResult results[] = {{"192.0.2.0", 9000}, + {"127.0.0.1", 9000}}; + struct fixture *f = data; + AddrinfoInjectSetResponse(0, 2, results); + LISTEN(RAFT_IOERR); + return MUNIT_OK; +} + +/* Error in bind of second IP address */ +TEST(tcp_listen, cannotBindSecond, setUp, tearDown, 0, NULL) +{ + const struct AddrinfoResult results[] = {{"127.0.0.1", 9000}, + {"192.0.2.0", 9000}}; + struct fixture *f = data; + AddrinfoInjectSetResponse(0, 2, results); + LISTEN(RAFT_IOERR); + return MUNIT_OK; +} + +/* Check error on general dns server failure */ +TEST(tcp_listen, resolveFailure, setUp, tearDown, 0, NULL) +{ + struct fixture *f = data; + AddrinfoInjectSetResponse(EAI_FAIL, 0, NULL); + LISTEN(RAFT_IOERR); + return MUNIT_OK; +} From 1eb4fd744e8c808e923f8ec57434437f08417324 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Tue, 20 Aug 2024 19:41:31 -0400 Subject: [PATCH 05/35] test: Rename PAGE_SIZE constant to avoid clash with system define Signed-off-by: Cole Miller --- test/integration/test_vfs.c | 14 +++++++------- test/unit/test_vfs2.c | 14 +++++++------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/test/integration/test_vfs.c b/test/integration/test_vfs.c index 3335c228c..61f80046d 100644 --- a/test/integration/test_vfs.c +++ b/test/integration/test_vfs.c @@ -88,7 +88,7 @@ static void tearDownRestorePendingByte(void *data) tearDown(data); } -#define PAGE_SIZE 512 +#define DB_PAGE_SIZE 512 #define PRAGMA(DB, COMMAND) \ _rv = sqlite3_exec(DB, "PRAGMA " COMMAND, NULL, NULL, NULL); \ @@ -216,12 +216,12 @@ struct tx if (_frames != NULL) { \ TX.page_numbers = \ munit_malloc(sizeof *TX.page_numbers * TX.n); \ - TX.frames = munit_malloc(PAGE_SIZE * TX.n); \ + TX.frames = munit_malloc(DB_PAGE_SIZE * TX.n); \ for (_i = 0; _i < TX.n; _i++) { \ dqlite_vfs_frame *_frame = &_frames[_i]; \ TX.page_numbers[_i] = _frame->page_number; \ - memcpy(TX.frames + _i * PAGE_SIZE, \ - _frame->data, PAGE_SIZE); \ + memcpy(TX.frames + _i * DB_PAGE_SIZE, \ + _frame->data, DB_PAGE_SIZE); \ sqlite3_free(_frame->data); \ } \ sqlite3_free(_frames); \ @@ -1502,7 +1502,7 @@ TEST(vfs, snapshotInitialDatabase, setUp, tearDown, 0, vfs_params) SNAPSHOT("1", snapshot); - munit_assert_int(snapshot.n, ==, PAGE_SIZE); + munit_assert_int(snapshot.n, ==, DB_PAGE_SIZE); page = snapshot.data; munit_assert_int(memcmp(&page[16], page_size, 2), ==, 0); @@ -1536,7 +1536,7 @@ TEST(vfs, snapshotAfterFirstTransaction, setUp, tearDown, 0, vfs_params) SNAPSHOT("1", snapshot); - munit_assert_int(snapshot.n, ==, PAGE_SIZE + 32 + (24 + PAGE_SIZE) * 2); + munit_assert_int(snapshot.n, ==, DB_PAGE_SIZE + 32 + (24 + DB_PAGE_SIZE) * 2); page = snapshot.data; munit_assert_int(memcmp(&page[16], page_size, 2), ==, 0); @@ -1571,7 +1571,7 @@ TEST(vfs, snapshotAfterCheckpoint, setUp, tearDown, 0, vfs_params) SNAPSHOT("1", snapshot); - munit_assert_int(snapshot.n, ==, PAGE_SIZE * 2); + munit_assert_int(snapshot.n, ==, DB_PAGE_SIZE * 2); page = snapshot.data; munit_assert_int(memcmp(&page[16], page_size, 2), ==, 0); diff --git a/test/unit/test_vfs2.c b/test/unit/test_vfs2.c index 635d7ce7f..961018e5f 100644 --- a/test/unit/test_vfs2.c +++ b/test/unit/test_vfs2.c @@ -14,8 +14,8 @@ #include #include -#define PAGE_SIZE 512 -#define PAGE_SIZE_STR "512" +#define DB_PAGE_SIZE 512 +#define DB_PAGE_SIZE_STR "512" SUITE(vfs2); @@ -105,7 +105,7 @@ TEST(vfs2, basic, set_up, tear_down, 0, NULL) munit_assert_int(rv, ==, SQLITE_OK); rv = sqlite3_exec(db, - "PRAGMA page_size=" PAGE_SIZE_STR ";" + "PRAGMA page_size=" DB_PAGE_SIZE_STR ";" "PRAGMA journal_mode=WAL;" "PRAGMA wal_autocheckpoint=0", NULL, NULL, NULL); @@ -209,7 +209,7 @@ TEST(vfs2, basic, set_up, tear_down, 0, NULL) return MUNIT_OK; } -#define WAL_SIZE_FROM_FRAMES(n) (32 + (24 + PAGE_SIZE) * (n)) +#define WAL_SIZE_FROM_FRAMES(n) (32 + (24 + DB_PAGE_SIZE) * (n)) static void make_wal_hdr(uint8_t *buf, uint32_t ckpoint_seqno, uint32_t salt1, uint32_t salt2) { @@ -220,7 +220,7 @@ static void make_wal_hdr(uint8_t *buf, uint32_t ckpoint_seqno, uint32_t salt1, u p += 4; BytePutBe32(3007000, p); p += 4; - BytePutBe32(PAGE_SIZE, p); + BytePutBe32(DB_PAGE_SIZE, p); p += 4; BytePutBe32(ckpoint_seqno, p); p += 4; @@ -269,7 +269,7 @@ TEST(vfs2, startup_one_nonempty, set_up, tear_down, 0, NULL) munit_assert_int(rv, ==, SQLITE_OK); tracef("setup..."); rv = sqlite3_exec(db, - "PRAGMA page_size=" PAGE_SIZE_STR ";" + "PRAGMA page_size=" DB_PAGE_SIZE_STR ";" "PRAGMA journal_mode=WAL;" "PRAGMA wal_autocheckpoint=0", NULL, NULL, NULL); @@ -309,7 +309,7 @@ TEST(vfs2, startup_both_nonempty, set_up, tear_down, 0, NULL) int rv = sqlite3_open(buf, &db); munit_assert_int(rv, ==, SQLITE_OK); rv = sqlite3_exec(db, - "PRAGMA page_size=" PAGE_SIZE_STR ";" + "PRAGMA page_size=" DB_PAGE_SIZE_STR ";" "PRAGMA journal_mode=WAL;" "PRAGMA wal_autocheckpoint=0", NULL, NULL, NULL); From 8737e068608479aa2e3842cf83aee8e452c985c9 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Tue, 20 Aug 2024 19:42:09 -0400 Subject: [PATCH 06/35] test: Don't call sqlite3_config to replace allocator after setup Signed-off-by: Cole Miller --- test/lib/heap.c | 67 +++++++++++++++---------------------------------- 1 file changed, 20 insertions(+), 47 deletions(-) diff --git a/test/lib/heap.c b/test/lib/heap.c index dba01591a..4db0cf2c8 100644 --- a/test/lib/heap.c +++ b/test/lib/heap.c @@ -1,3 +1,4 @@ +#include #include #include "fault.h" @@ -97,15 +98,6 @@ static void mem_wrap(sqlite3_mem_methods *m, sqlite3_mem_methods *wrap) wrap->pAppData = &memFault; } -/* Unwrap the given faulty memory management instance returning the original - * one. */ -static void mem_unwrap(sqlite3_mem_methods *wrap, sqlite3_mem_methods *m) -{ - (void)wrap; - - *m = memFault.m; -} - /* Get the current number of outstanding malloc()'s without a matching free() * and the total number of used memory. */ static void mem_stats(int *malloc_count, int *memory_used) @@ -126,22 +118,12 @@ static void mem_stats(int *malloc_count, int *memory_used) } } -/* Ensure we're starting from a clean memory state with no allocations and - * optionally inject malloc failures. */ -void test_heap_setup(const MunitParameter params[], void *user_data) +static void replace_sqlite_heap(void) { - int malloc_count; - int memory_used; - const char *fault_delay; - const char *fault_repeat; sqlite3_mem_methods mem; sqlite3_mem_methods mem_fault; int rc; - (void)params; - (void)user_data; - - /* Install the faulty malloc implementation */ rc = sqlite3_config(SQLITE_CONFIG_GETMALLOC, &mem); if (rc != SQLITE_OK) { munit_errorf("can't get default mem: %s", sqlite3_errstr(rc)); @@ -153,6 +135,24 @@ void test_heap_setup(const MunitParameter params[], void *user_data) if (rc != SQLITE_OK) { munit_errorf("can't set faulty mem: %s", sqlite3_errstr(rc)); } +} + +/* Ensure we're starting from a clean memory state with no allocations and + * optionally inject malloc failures. */ +void test_heap_setup(const MunitParameter params[], void *user_data) +{ + int malloc_count; + int memory_used; + const char *fault_delay; + const char *fault_repeat; + int rc; + + (void)params; + (void)user_data; + + static pthread_once_t once = PTHREAD_ONCE_INIT; + rc = pthread_once(&once, replace_sqlite_heap); + munit_assert_int(rc, ==, 0); /* Check that memory is clean. */ mem_stats(&malloc_count, &memory_used); @@ -177,34 +177,7 @@ void test_heap_setup(const MunitParameter params[], void *user_data) /* Ensure we're starting leaving a clean memory behind. */ void test_heap_tear_down(void *data) { - sqlite3_mem_methods mem; - sqlite3_mem_methods mem_fault; - int rc; - (void)data; - - int malloc_count; - int memory_used; - - mem_stats(&malloc_count, &memory_used); - if (malloc_count > 0 || memory_used > 0) { - /* munit_errorf( - "teardown memory:\n bytes: %11d\n allocations: %5d\n", - memory_used, malloc_count); */ - } - - /* Restore default memory management. */ - rc = sqlite3_config(SQLITE_CONFIG_GETMALLOC, &mem_fault); - if (rc != SQLITE_OK) { - munit_errorf("can't get faulty mem: %s", sqlite3_errstr(rc)); - } - - mem_unwrap(&mem_fault, &mem); - - rc = sqlite3_config(SQLITE_CONFIG_MALLOC, &mem); - if (rc != SQLITE_OK) { - munit_errorf("can't reset default mem: %s", sqlite3_errstr(rc)); - } } void test_heap_fault_config(int delay, int repeat) From bb328c242e85bcd72cb2f3a98f2deb295a056142 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Tue, 20 Aug 2024 19:42:37 -0400 Subject: [PATCH 07/35] test: Remove include of execinfo.h Signed-off-by: Cole Miller --- test/lib/runner.h | 1 - 1 file changed, 1 deletion(-) diff --git a/test/lib/runner.h b/test/lib/runner.h index 784066321..f798c2bb8 100644 --- a/test/lib/runner.h +++ b/test/lib/runner.h @@ -4,7 +4,6 @@ #define TEST_RUNNER_H #include -#include #include #include From 98284eaa8b2134c499f94f5c16ff70a0f711aa64 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Mon, 26 Aug 2024 17:49:33 -0400 Subject: [PATCH 08/35] Suppress generation of unwind tables Signed-off-by: Cole Miller --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 6bc4afebc..d7bc5b379 100644 --- a/configure.ac +++ b/configure.ac @@ -99,7 +99,7 @@ CC_CHECK_FLAGS_APPEND([AM_CFLAGS],[CFLAGS],[ \ -fexceptions \ -fstack-clash-protection \ -fstack-protector-strong \ - -fasynchronous-unwind-tables \ + -fno-exceptions \ -fdiagnostics-show-option \ -Wall \ -Wextra \ From 8769923d2d095dee78d72858c269ceff3da1a24d Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Mon, 26 Aug 2024 18:36:23 -0400 Subject: [PATCH 09/35] Add static build job on Ubuntu Signed-off-by: Cole Miller --- .github/workflows/static.yml | 21 +++++ configure.ac | 3 +- contrib/build-static.sh | 149 +++++++++++++++++++++++++++++++++++ 3 files changed, 171 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/static.yml create mode 100755 contrib/build-static.sh diff --git a/.github/workflows/static.yml b/.github/workflows/static.yml new file mode 100644 index 000000000..d35b15899 --- /dev/null +++ b/.github/workflows/static.yml @@ -0,0 +1,21 @@ +name: CI Tests (musl build) + +on: + - push + - pull_request + +jobs: + build-and-test: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v4 + + - name: Install dependencies + run: | + sudo apt install -y build-essential automake libtool gettext autopoint tclsh tcl libsqlite3-dev pkg-config git + + - name: Build and test + run: | + export LIBDQLITE_TRACE=1 + contrib/build-static.sh diff --git a/configure.ac b/configure.ac index d7bc5b379..bddf38285 100644 --- a/configure.ac +++ b/configure.ac @@ -6,8 +6,7 @@ AC_CONFIG_AUX_DIR([ac]) AM_INIT_AUTOMAKE([subdir-objects -Wall -Werror -Wno-portability foreign]) AM_SILENT_RULES([yes]) -# Without this line, AC_PROG_CC boneheadedly adds `-g -O2` to our CFLAGS. -AC_SUBST(CFLAGS, "") +AC_SUBST(AM_CFLAGS) AC_PROG_CC AC_USE_SYSTEM_EXTENSIONS diff --git a/contrib/build-static.sh b/contrib/build-static.sh new file mode 100755 index 000000000..54a5d8366 --- /dev/null +++ b/contrib/build-static.sh @@ -0,0 +1,149 @@ +#!/bin/bash -xeu + +DIR="${DIR:=$(realpath `dirname "${0}"`)}" + +REPO_MUSL="https://git.launchpad.net/musl" +REPO_LIBTIRPC="https://salsa.debian.org/debian/libtirpc.git" +REPO_LIBNSL="https://github.com/thkukuk/libnsl.git" +REPO_LIBUV="https://github.com/libuv/libuv.git" +REPO_LIBLZ4="https://github.com/lz4/lz4.git" +REPO_SQLITE="https://github.com/sqlite/sqlite.git" +REPO_DQLITE="https://github.com/cole-miller/dqlite.git" + +TAG_MUSL="v1.2.4" +TAG_LIBTIRPC="upstream/1.3.3" +TAG_LIBNSL="v2.0.1" +TAG_LIBUV="v1.48.0" +TAG_LIBLZ4="v1.9.4" +TAG_SQLITE="version-3.45.1" +TAG_DQLITE="musl-ci" + +BUILD_DIR="${DIR}/build" +INSTALL_DIR="${DIR}/prefix" +mkdir -p "${BUILD_DIR}" "${INSTALL_DIR}" "${INSTALL_DIR}/lib" "${INSTALL_DIR}/include" +BUILD_DIR="$(realpath "${BUILD_DIR}")" +INSTALL_DIR="$(realpath "${INSTALL_DIR}")" + +export CFLAGS="" +MACHINE_TYPE="$(uname -m)" +if [ "${MACHINE_TYPE}" = "ppc64le" ]; then + MACHINE_TYPE="powerpc64le" + export CFLAGS="-mlong-double-64" +fi +export PKG_CONFIG_PATH="${INSTALL_DIR}/lib/pkgconfig" + +# build musl +if [ ! -f "${INSTALL_DIR}/musl/bin/musl-gcc" ]; then + ( + cd "${BUILD_DIR}" + rm -rf musl + git clone "${REPO_MUSL}" --depth 1 --branch "${TAG_MUSL}" musl + cd musl + ./configure --prefix="${INSTALL_DIR}/musl" + make -j + make -j install + + # missing musl header files + ln -s "/usr/include/${MACHINE_TYPE}-linux-gnu/sys/queue.h" "${INSTALL_DIR}/musl/include/sys/queue.h" || true + ln -s "/usr/include/${MACHINE_TYPE}-linux-gnu/asm" "${INSTALL_DIR}/musl/include/asm" || true + ln -s /usr/include/asm-generic "${INSTALL_DIR}/musl/include/asm-generic" || true + ln -s /usr/include/linux "${INSTALL_DIR}/musl/include/linux" || true + ) +fi + +export PATH="${PATH}:${INSTALL_DIR}/musl/bin" +export CFLAGS="${CFLAGS} -isystem ${INSTALL_DIR}/musl/include" +export CC=musl-gcc +export LDFLAGS=-static + +# build libtirpc +if [ ! -f "${BUILD_DIR}/libtirpc/src/libtirpc.la" ]; then + ( + cd "${BUILD_DIR}" + rm -rf libtirpc + git clone "${REPO_LIBTIRPC}" --depth 1 --branch "${TAG_LIBTIRPC}" + cd libtirpc + chmod +x autogen.sh + ./autogen.sh + ./configure --disable-shared --disable-gssapi --prefix="${INSTALL_DIR}" + make -j install + ) +fi + +# build libnsl +if [ ! -f "${BUILD_DIR}/libnsl/src/libnsl.la" ]; then + ( + cd "${BUILD_DIR}" + rm -rf libnsl + git clone "${REPO_LIBNSL}" --depth 1 --branch "${TAG_LIBNSL}" + cd libnsl + ./autogen.sh + autoreconf -i + autoconf + ./configure --disable-shared --prefix="${INSTALL_DIR}" + make -j install + ) +fi + +# build libuv +if [ ! -f "${BUILD_DIR}/libuv/libuv.la" ]; then + ( + cd "${BUILD_DIR}" + rm -rf libuv + git clone "${REPO_LIBUV}" --depth 1 --branch "${TAG_LIBUV}" + cd libuv + ./autogen.sh + ./configure --disable-shared --prefix="${INSTALL_DIR}" + make -j install + ) +fi + +# build liblz4 +if [ ! -f "${BUILD_DIR}/lz4/lib/liblz4.a" ] || [ ! -f "${BUILD_DIR}/lz4/lib/liblz4.so" ]; then + ( + cd "${BUILD_DIR}" + rm -rf lz4 + git clone "${REPO_LIBLZ4}" --depth 1 --branch "${TAG_LIBLZ4}" + cd lz4 + make install -j PREFIX="${INSTALL_DIR}" BUILD_SHARED=no + ) +fi + +# build sqlite3 +if [ ! -f "${BUILD_DIR}/sqlite/libsqlite3.la" ]; then + ( + cd "${BUILD_DIR}" + rm -rf sqlite + git clone "${REPO_SQLITE}" --depth 1 --branch "${TAG_SQLITE}" + cd sqlite + ./configure --disable-shared --disable-readline --prefix="${INSTALL_DIR}" \ + CFLAGS="${CFLAGS} -DSQLITE_ENABLE_DBSTAT_VTAB=1" + make install -j BCC="${CC} -g -O2 ${CFLAGS} ${LDFLAGS}" + ) +fi + +# build dqlite +if [ ! -f "${BUILD_DIR}/dqlite/libdqlite.la" ]; then + ( + cd "${BUILD_DIR}" + rm -rf dqlite + git clone "${REPO_DQLITE}" --depth 1 --branch "${TAG_DQLITE}" + cd dqlite + autoreconf -i + ./configure --disable-shared --enable-build-raft --prefix="${INSTALL_DIR}" + + # Don't run the raft addrinfo tests since they rely on libc being + # dynamically linked. + bins="unit-test integration-test \ + raft-core-fuzzy-test \ + raft-core-integration-test \ + raft-core-unit-test \ + raft-uv-integration-test \ + raft-uv-unit-test" + make -j LDFLAGS=-all-static $bins + for bin in $bins + do LIBDQLITE_TRACE=1 ./$bin || touch any-failed + done + test '!' -e any-failed + ) +fi From f9d08661a3440c981dc995de828a5517cb724f2a Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Wed, 28 Aug 2024 18:18:01 -0400 Subject: [PATCH 10/35] Run addrinfo binary in normal CI test job Signed-off-by: Cole Miller --- .github/workflows/build-and-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 23ac612ee..cf6c3ac2b 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -52,7 +52,7 @@ jobs: for bin in unit-test integration-test \ raft-core-fuzzy-test raft-core-integration-test \ raft-core-unit-test raft-uv-integration-test \ - raft-uv-unit-test + raft-uv-unit-test raft-addrinfo-integration-test do ./$bin || touch any-failed done 2>&1 | tee -a test-suite.log test '!' -e any-failed From 69079c32a4d62a60a5ea6e68bf4bf5fc1e1d5a89 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 29 Aug 2024 12:40:46 -0400 Subject: [PATCH 11/35] Makefile.am: better test support Signed-off-by: Cole Miller --- .github/workflows/build-and-test.yml | 18 +++++------------- Makefile.am | 5 +++++ contrib/build-static.sh | 15 ++------------- 3 files changed, 12 insertions(+), 26 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index cf6c3ac2b..0fa728246 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -36,25 +36,17 @@ jobs: autoreconf -i ./configure --enable-debug --enable-code-coverage --enable-sanitize \ --enable-build-raft --enable-dqlite-next=${{ matrix.dqlite-next }} - make -j4 unit-test integration-test \ - raft-core-fuzzy-test \ - raft-core-integration-test \ - raft-core-unit-test \ - raft-uv-integration-test \ - raft-addrinfo-integration-test \ - raft-uv-unit-test + make -j$(nproc) check-norun - name: Test env: CC: ${{ matrix.compiler }} LIBDQLITE_TRACE: 1 run: | - for bin in unit-test integration-test \ - raft-core-fuzzy-test raft-core-integration-test \ - raft-core-unit-test raft-uv-integration-test \ - raft-uv-unit-test raft-addrinfo-integration-test - do ./$bin || touch any-failed - done 2>&1 | tee -a test-suite.log + # Temporary measure: don't use `make check` because then we won't see + # the partial test output when there's a hang in CI. + bins="$(make print-test-programs)" + for bin in $bins; do ./$bin || touch any-failed; done 2>&1 | tee -a test-suite.log test '!' -e any-failed - name: OnTestFailure diff --git a/Makefile.am b/Makefile.am index 9c1285acf..d7de8d820 100644 --- a/Makefile.am +++ b/Makefile.am @@ -363,6 +363,11 @@ endif TESTS = $(check_PROGRAMS) +check-norun: $(check_PROGRAMS) + +print-test-binaries: + @echo '$(check_PROGRAMS)' + if CODE_COVERAGE_ENABLED include $(top_srcdir)/aminclude_static.am diff --git a/contrib/build-static.sh b/contrib/build-static.sh index 54a5d8366..726826430 100755 --- a/contrib/build-static.sh +++ b/contrib/build-static.sh @@ -132,18 +132,7 @@ if [ ! -f "${BUILD_DIR}/dqlite/libdqlite.la" ]; then autoreconf -i ./configure --disable-shared --enable-build-raft --prefix="${INSTALL_DIR}" - # Don't run the raft addrinfo tests since they rely on libc being - # dynamically linked. - bins="unit-test integration-test \ - raft-core-fuzzy-test \ - raft-core-integration-test \ - raft-core-unit-test \ - raft-uv-integration-test \ - raft-uv-unit-test" - make -j LDFLAGS=-all-static $bins - for bin in $bins - do LIBDQLITE_TRACE=1 ./$bin || touch any-failed - done - test '!' -e any-failed + make -j LDFLAGS=-all-static check-norun + make check ) fi From 86b5f086e02424d683a6d2f732c034391ced45fe Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 29 Aug 2024 14:07:05 -0400 Subject: [PATCH 12/35] Build configuration for static deps Signed-off-by: Cole Miller --- Makefile.am | 5 +++++ configure.ac | 6 ++++++ contrib/build-static.sh | 5 ++--- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Makefile.am b/Makefile.am index d7de8d820..b0cd6915e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -28,6 +28,11 @@ AM_CFLAGS += -DDQLITE_ASSERT_WITH_BACKTRACE -DRAFT_ASSERT_WITH_BACKTRACE AM_LDFLAGS += -lbacktrace endif +if WITH_STATIC_DEPS +AM_CFLAGS += -DDQLITE_STATIC_LIBC +AM_LDFLAGS += -all-static +endif + include_HEADERS = include/dqlite.h basic_dqlite_sources = \ diff --git a/configure.ac b/configure.ac index bddf38285..fbbe048de 100644 --- a/configure.ac +++ b/configure.ac @@ -42,6 +42,12 @@ AC_ARG_ENABLE(dqlite-next, AS_HELP_STRING([--enable-dqlite-next[=ARG]], [build w AM_CONDITIONAL(DQLITE_NEXT_ENABLED, test "x$enable_dqlite_next" = "xyes") AS_IF([test "x$enable_build_raft" != "xyes" -a "x$enable_dqlite_next" = "xyes"], [AC_MSG_ERROR([dqlite-next requires bundled raft])], []) +AC_ARG_WITH(static-deps, + AS_HELP_STRING([--with-static-deps[=ARG]], + [skip building a shared library and link test binaries statically]) + [], [with_static_deps=no]) +AM_CONDITIONAL(WITH_STATIC_DEPS, test "x$with_static_deps" != "xno") + # Whether to enable code coverage. AX_CODE_COVERAGE diff --git a/contrib/build-static.sh b/contrib/build-static.sh index 726826430..ae3f33e23 100755 --- a/contrib/build-static.sh +++ b/contrib/build-static.sh @@ -130,9 +130,8 @@ if [ ! -f "${BUILD_DIR}/dqlite/libdqlite.la" ]; then git clone "${REPO_DQLITE}" --depth 1 --branch "${TAG_DQLITE}" cd dqlite autoreconf -i - ./configure --disable-shared --enable-build-raft --prefix="${INSTALL_DIR}" - - make -j LDFLAGS=-all-static check-norun + ./configure --enable-build-raft --with-static-deps --prefix="${INSTALL_DIR}" + make -j check-norun make check ) fi From b253ea427714e2fb0980c580ec93f0c12b35c775 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 29 Aug 2024 14:18:27 -0400 Subject: [PATCH 13/35] Revert "test: Split raft uv tests that rely on mocking getaddrinfo" This reverts commit 87e8e6bf62d44edbbf66394d203ba16daf4c0f7f. --- Makefile.am | 60 ++-- test/raft/integration/main_addrinfo.c | 3 - test/raft/integration/test_uv_tcp_connect.c | 53 ++++ .../test_uv_tcp_connect_addrinfo.c | 231 --------------- test/raft/integration/test_uv_tcp_listen.c | 80 ++++++ .../integration/test_uv_tcp_listen_addrinfo.c | 268 ------------------ 6 files changed, 157 insertions(+), 538 deletions(-) delete mode 100644 test/raft/integration/main_addrinfo.c delete mode 100644 test/raft/integration/test_uv_tcp_connect_addrinfo.c delete mode 100644 test/raft/integration/test_uv_tcp_listen_addrinfo.c diff --git a/Makefile.am b/Makefile.am index b0cd6915e..f0d131e3d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -51,6 +51,8 @@ basic_dqlite_sources = \ src/lib/addr.c \ src/lib/buffer.c \ src/lib/fs.c \ + src/lib/sm.c \ + src/lib/threadpool.c \ src/lib/transport.c \ src/logger.c \ src/message.c \ @@ -63,6 +65,7 @@ basic_dqlite_sources = \ src/roles.c \ src/server.c \ src/stmt.c \ + src/tracing.c \ src/transport.c \ src/translate.c \ src/tuple.c \ @@ -73,7 +76,6 @@ lib_LTLIBRARIES = libdqlite.la libdqlite_la_CFLAGS = $(AM_CFLAGS) -fvisibility=hidden -DRAFT_API='' libdqlite_la_LDFLAGS = $(AM_LDFLAGS) -version-info 0:1:0 libdqlite_la_SOURCES = $(basic_dqlite_sources) -libdqlite_la_LIBADD = libcommon.la if BUILD_RAFT_ENABLED libraft_la_SOURCES = \ @@ -131,10 +133,9 @@ libraft_la_SOURCES = \ libdqlite_la_SOURCES += $(libraft_la_SOURCES) endif # BUILD_RAFT_ENABLED -# When adding a new test binary, make sure to update CI. check_PROGRAMS = unit-test integration-test -check_LTLIBRARIES = libtest.la libcommon.la +check_LTLIBRARIES = libtest.la libtest_la_CFLAGS = $(AM_CFLAGS) -DMUNIT_TEST_NAME_LEN=60 -Wno-unknown-warning-option -Wno-unused-result -Wno-conversion -Wno-uninitialized -Wno-maybe-uninitialized -Wno-strict-prototypes -Wno-old-style-definition libtest_la_SOURCES = \ @@ -149,16 +150,6 @@ libtest_la_SOURCES = \ test/lib/sqlite.c \ test/lib/uv.c -# libcommon is the destination for code that is used by both the raft -# half and the dqlite half of the codebase, but isn't part of the raft -# API surface. It's also used to make these bits of code accessible to -# the unit tests without compiling the same few files repeatedly. -libcommon_la_CFLAGS = $(AM_CFLAGS) -libcommon_la_SOURCES = \ - src/tracing.c \ - src/lib/sm.c \ - src/lib/threadpool.c - unit_test_SOURCES = $(basic_dqlite_sources) unit_test_SOURCES += \ test/test_error.c \ @@ -186,7 +177,7 @@ unit_test_SOURCES += \ test/unit/main.c unit_test_CFLAGS = $(AM_CFLAGS) -Wno-unknown-warning-option -Wno-uninitialized -Wno-maybe-uninitialized -Wno-float-equal -Wno-conversion unit_test_LDFLAGS = $(AM_LDFLAGS) -unit_test_LDADD = libcommon.la libtest.la +unit_test_LDADD = libtest.la if BUILD_RAFT_ENABLED unit_test_LDADD += libraft.la @@ -209,16 +200,15 @@ integration_test_LDADD = libtest.la libdqlite.la if BUILD_RAFT_ENABLED check_LTLIBRARIES += libraft.la -# When adding a new test binary, make sure to update CI. check_PROGRAMS += \ raft-core-unit-test \ raft-core-integration-test \ raft-uv-unit-test \ raft-uv-integration-test \ - raft-addrinfo-integration-test \ raft-core-fuzzy-test libtest_la_SOURCES += \ + test/raft/lib/addrinfo.c \ test/raft/lib/fault.c \ test/raft/lib/fsm.c \ test/raft/lib/heap.c \ @@ -232,10 +222,12 @@ libtest_la_SOURCES += \ libraft_la_CFLAGS = $(AM_CFLAGS) libraft_la_LDFLAGS = $(UV_LIBS) -libraft_la_LIBADD = libcommon.la raft_core_unit_test_SOURCES = \ $(libraft_la_SOURCES) \ + src/lib/sm.c \ + src/lib/threadpool.c \ + src/tracing.c \ test/raft/unit/main_core.c \ test/raft/unit/test_byte.c \ test/raft/unit/test_compress.c \ @@ -247,9 +239,12 @@ raft_core_unit_test_SOURCES = \ test/raft/unit/test_snapshot.c raft_core_unit_test_CFLAGS = $(AM_CFLAGS) -Wno-conversion -raft_core_unit_test_LDADD = libcommon.la libtest.la +raft_core_unit_test_LDADD = libtest.la raft_core_integration_test_SOURCES = \ + src/tracing.c \ + src/lib/sm.c \ + src/lib/threadpool.c \ test/raft/integration/main_core.c \ test/raft/integration/test_apply.c \ test/raft/integration/test_assign.c \ @@ -274,6 +269,9 @@ raft_core_integration_test_LDFLAGS = -no-install raft_core_integration_test_LDADD = libtest.la libraft.la raft_core_fuzzy_test_SOURCES = \ + src/lib/sm.c \ + src/lib/threadpool.c \ + src/tracing.c \ test/raft/fuzzy/main_core.c \ test/raft/fuzzy/test_election.c \ test/raft/fuzzy/test_liveness.c \ @@ -284,6 +282,7 @@ raft_core_fuzzy_test_LDFLAGS = -no-install raft_core_fuzzy_test_LDADD = libtest.la libraft.la raft_uv_unit_test_SOURCES = \ + src/tracing.c \ src/raft/err.c \ src/raft/heap.c \ src/raft/syscall.c \ @@ -295,12 +294,16 @@ raft_uv_unit_test_SOURCES = \ test/raft/unit/test_uv_os.c \ test/raft/unit/test_uv_writer.c raft_uv_unit_test_CFLAGS = $(AM_CFLAGS) -Wno-conversion -raft_uv_unit_test_LDADD = libcommon.la libtest.la $(UV_LIBS) +raft_uv_unit_test_LDADD = libtest.la $(UV_LIBS) # The integration/uv test is not linked to libraft, but built # directly against the libraft sources in order to test some # non-visible, non-API functions. raft_uv_integration_test_SOURCES = \ + $(libraft_la_SOURCES) \ + src/tracing.c \ + src/lib/sm.c \ + src/lib/threadpool.c \ test/raft/integration/main_uv.c \ test/raft/integration/test_uv_init.c \ test/raft/integration/test_uv_append.c \ @@ -318,22 +321,7 @@ raft_uv_integration_test_SOURCES = \ test/raft/integration/test_uv_work.c raft_uv_integration_test_CFLAGS = $(AM_CFLAGS) -Wno-type-limits -Wno-conversion raft_uv_integration_test_LDFLAGS = -no-install -raft_uv_integration_test_LDADD = libtest.la libraft.la $(UV_LIBS) - -# Some of the uv integration tests are broken out into a separate -# binary because they use a trick to override libc's getaddrinfo -# with a mock implementation. When libc is statically linked (a use-case -# that we care about), this trick doesn't work, and prevents any -# test in the same binary that calls getaddrinfo from working -# correctly. -raft_addrinfo_integration_test_SOURCES = \ - test/raft/lib/addrinfo.c \ - test/raft/integration/test_uv_tcp_connect_addrinfo.c \ - test/raft/integration/test_uv_tcp_listen_addrinfo.c \ - test/raft/integration/main_addrinfo.c -raft_addrinfo_integration_test_CFLAGS = $(AM_CFLAGS) -Wno-type-limits -Wno-conversion -raft_addrinfo_integration_test_LDFLAGS = -no-install -raft_addrinfo_integration_test_LDADD = libtest.la libraft.la $(UV_LIBS) +raft_uv_integration_test_LDADD = libtest.la $(UV_LIBS) if LZ4_AVAILABLE libdqlite_la_CFLAGS += -DLZ4_AVAILABLE $(LZ4_CFLAGS) @@ -361,7 +349,7 @@ libsqlite3_la_SOURCES = sqlite3.c libsqlite3_la_CFLAGS = -g3 unit_test_LDADD += libsqlite3.la -libdqlite_la_LIBADD += libsqlite3.la +libdqlite_la_LIBADD = libsqlite3.la else AM_LDFLAGS += $(SQLITE_LIBS) endif diff --git a/test/raft/integration/main_addrinfo.c b/test/raft/integration/main_addrinfo.c deleted file mode 100644 index 07476ef73..000000000 --- a/test/raft/integration/main_addrinfo.c +++ /dev/null @@ -1,3 +0,0 @@ -#include "../lib/runner.h" - -RUNNER("addrinfo") diff --git a/test/raft/integration/test_uv_tcp_connect.c b/test/raft/integration/test_uv_tcp_connect.c index b6395c6f0..7efc68c60 100644 --- a/test/raft/integration/test_uv_tcp_connect.c +++ b/test/raft/integration/test_uv_tcp_connect.c @@ -1,5 +1,6 @@ #include "../../../src/raft.h" #include "../../../src/raft.h" +#include "../lib/addrinfo.h" #include "../lib/heap.h" #include "../lib/loop.h" #include "../lib/runner.h" @@ -125,6 +126,7 @@ static void *setUpDeps(const MunitParameter params[], { struct fixture *f = munit_malloc(sizeof *f); int rv; + SET_UP_ADDRINFO; SET_UP_HEAP; SETUP_LOOP; SETUP_TCP_SERVER; @@ -142,6 +144,7 @@ static void tearDownDeps(void *data) TEAR_DOWN_TCP_SERVER; TEAR_DOWN_LOOP; TEAR_DOWN_HEAP; + TEAR_DOWN_ADDRINFO; free(f); } @@ -188,6 +191,29 @@ TEST(tcp_connect, connectByName, setUp, tearDown, 0, NULL) return MUNIT_OK; } +/* Successfully connect to the peer by first IP */ +TEST(tcp_connect, firstIP, setUp, tearDown, 0, NULL) +{ + struct fixture *f = data; + const struct AddrinfoResult results[] = {{"127.0.0.1", TCP_SERVER_PORT}, + {"192.0.2.0", 6666}}; + AddrinfoInjectSetResponse(0, 2, results); + CONNECT(2, "any-host"); + return MUNIT_OK; +} + +/* Successfully connect to the peer by second IP */ +TEST(tcp_connect, secondIP, setUp, tearDown, 0, NULL) +{ + struct fixture *f = data; + const struct AddrinfoResult results[] = {{"127.0.0.1", .6666}, + {"127.0.0.1", TCP_SERVER_PORT}}; + + AddrinfoInjectSetResponse(0, 2, results); + CONNECT(2, "any-host"); + return MUNIT_OK; +} + /* The peer has shutdown */ TEST(tcp_connect, refused, setUp, tearDown, 0, NULL) { @@ -303,3 +329,30 @@ TEST(tcp_connect, closeDuringConnectAbort, setUp, tearDownDeps, 0, NULL) CLOSE_WAIT; return MUNIT_OK; } + +/* The transport gets closed right after the first connection attempt failed, + * while doing a second connection attempt. */ +TEST(tcp_connect, closeDuringSecondConnect, setUp, tearDownDeps, 0, NULL) +{ + struct fixture *f = data; + struct uv_check_s check; + int rv; + const struct AddrinfoResult results[] = {{"127.0.0.1", .6666}, + {"127.0.0.1", TCP_SERVER_PORT}}; + + AddrinfoInjectSetResponse(0, 2, results); + + /* Use a check handle in order to close the transport in the same loop + * iteration where the second connection attempt occurs. */ + rv = uv_check_init(&f->loop, &check); + munit_assert_int(rv, ==, 0); + check.data = f; + CONNECT_REQ(2, "any-host", 0, RAFT_CANCELED); + /* Successfull DNS lookup will initiate async connect */ + LOOP_RUN(1); + uv_check_start(&check, checkCb); + LOOP_RUN(1); + LOOP_RUN_UNTIL(&_result.done); + CLOSE_WAIT; + return MUNIT_OK; +} diff --git a/test/raft/integration/test_uv_tcp_connect_addrinfo.c b/test/raft/integration/test_uv_tcp_connect_addrinfo.c deleted file mode 100644 index 969b24ead..000000000 --- a/test/raft/integration/test_uv_tcp_connect_addrinfo.c +++ /dev/null @@ -1,231 +0,0 @@ -#include "../../../src/raft.h" -#include "../../../src/raft.h" -#include "../lib/addrinfo.h" -#include "../lib/heap.h" -#include "../lib/loop.h" -#include "../lib/runner.h" -#include "../lib/tcp.h" - -/****************************************************************************** - * - * Fixture with a TCP-based raft_uv_transport. - * - *****************************************************************************/ - -struct fixture -{ - FIXTURE_HEAP; - FIXTURE_LOOP; - FIXTURE_TCP_SERVER; - struct raft_uv_transport transport; - bool closed; -}; - -/****************************************************************************** - * - * Helper macros - * - *****************************************************************************/ - -struct result -{ - int status; - bool done; -}; - -static void closeCb(struct raft_uv_transport *transport) -{ - struct fixture *f = transport->data; - f->closed = true; -} - -static void connectCbAssertResult(struct raft_uv_connect *req, - struct uv_stream_s *stream, - int status) -{ - struct result *result = req->data; - munit_assert_int(status, ==, result->status); - if (status == 0) { - uv_close((struct uv_handle_s *)stream, (uv_close_cb)raft_free); - } - result->done = true; -} - -#define INIT \ - do { \ - int _rv; \ - _rv = f->transport.init(&f->transport, 1, "127.0.0.1:9000"); \ - munit_assert_int(_rv, ==, 0); \ - f->transport.data = f; \ - f->closed = false; \ - } while (0) - -#define CLOSE_SUBMIT \ - munit_assert_false(f->closed); \ - f->transport.close(&f->transport, closeCb); - -#define CLOSE_WAIT LOOP_RUN_UNTIL(&f->closed) -#define CLOSE \ - CLOSE_SUBMIT; \ - CLOSE_WAIT - -#define CONNECT_REQ(ID, ADDRESS, RV, STATUS) \ - struct raft_uv_connect _req; \ - struct result _result = {STATUS, false}; \ - int _rv; \ - _req.data = &_result; \ - _rv = f->transport.connect(&f->transport, &_req, ID, ADDRESS, \ - connectCbAssertResult); \ - munit_assert_int(_rv, ==, RV) - -/* Try to submit a connect request and assert that the given error code and - * message are returned. */ -#define CONNECT_ERROR(ID, ADDRESS, RV, ERRMSG) \ - { \ - CONNECT_REQ(ID, ADDRESS, RV /* rv */, 0 /* status */); \ - munit_assert_string_equal(f->transport.errmsg, ERRMSG); \ - } - -/* Submit a connect request with the given parameters and wait for the operation - * to successfully complete. */ -#define CONNECT(ID, ADDRESS) \ - { \ - CONNECT_REQ(ID, ADDRESS, 0 /* rv */, 0 /* status */); \ - LOOP_RUN_UNTIL(&_result.done); \ - } - -/* Submit a connect request with the given parameters and wait for the operation - * to fail with the given code and message. */ -#define CONNECT_FAILURE(ID, ADDRESS, STATUS, ERRMSG) \ - { \ - CONNECT_REQ(ID, ADDRESS, 0 /* rv */, STATUS); \ - LOOP_RUN_UNTIL(&_result.done); \ - munit_assert_string_equal(f->transport.errmsg, ERRMSG); \ - } - -/* Submit a connect request with the given parameters, close the transport after - * N loop iterations and assert that the request got canceled. */ -#define CONNECT_CLOSE(ID, ADDRESS, N) \ - { \ - CONNECT_REQ(ID, ADDRESS, 0 /* rv */, RAFT_CANCELED); \ - LOOP_RUN(N); \ - CLOSE_SUBMIT; \ - munit_assert_false(_result.done); \ - LOOP_RUN_UNTIL(&_result.done); \ - CLOSE_WAIT; \ - } - -/****************************************************************************** - * - * Set up and tear down. - * - *****************************************************************************/ - -static void *setUpDeps(const MunitParameter params[], - MUNIT_UNUSED void *user_data) -{ - struct fixture *f = munit_malloc(sizeof *f); - int rv; - SET_UP_ADDRINFO; - SET_UP_HEAP; - SETUP_LOOP; - SETUP_TCP_SERVER; - f->transport.version = 1; - rv = raft_uv_tcp_init(&f->transport, &f->loop); - munit_assert_int(rv, ==, 0); - return f; -} - -static void tearDownDeps(void *data) -{ - struct fixture *f = data; - LOOP_STOP; - raft_uv_tcp_close(&f->transport); - TEAR_DOWN_TCP_SERVER; - TEAR_DOWN_LOOP; - TEAR_DOWN_HEAP; - TEAR_DOWN_ADDRINFO; - free(f); -} - -static void *setUp(const MunitParameter params[], void *user_data) -{ - struct fixture *f = setUpDeps(params, user_data); - INIT; - return f; -} - -static void tearDown(void *data) -{ - struct fixture *f = data; - CLOSE; - tearDownDeps(f); -} - -/****************************************************************************** - * - * raft_uv_transport->connect() - * - *****************************************************************************/ - -#define BOGUS_ADDRESS "127.0.0.1:6666" -#define INVALID_ADDRESS "500.0.0.1:6666" - -SUITE(tcp_connect) - -/* Successfully connect to the peer by first IP */ -TEST(tcp_connect, firstIP, setUp, tearDown, 0, NULL) -{ - struct fixture *f = data; - const struct AddrinfoResult results[] = {{"127.0.0.1", TCP_SERVER_PORT}, - {"192.0.2.0", 6666}}; - AddrinfoInjectSetResponse(0, 2, results); - CONNECT(2, "any-host"); - return MUNIT_OK; -} - -/* Successfully connect to the peer by second IP */ -TEST(tcp_connect, secondIP, setUp, tearDown, 0, NULL) -{ - struct fixture *f = data; - const struct AddrinfoResult results[] = {{"127.0.0.1", .6666}, - {"127.0.0.1", TCP_SERVER_PORT}}; - - AddrinfoInjectSetResponse(0, 2, results); - CONNECT(2, "any-host"); - return MUNIT_OK; -} - -static void checkCb(struct uv_check_s *check) -{ - struct fixture *f = check->data; - CLOSE_SUBMIT; - uv_close((struct uv_handle_s *)check, NULL); -} - -/* The transport gets closed right after the first connection attempt failed, - * while doing a second connection attempt. */ -TEST(tcp_connect, closeDuringSecondConnect, setUp, tearDownDeps, 0, NULL) -{ - struct fixture *f = data; - struct uv_check_s check; - int rv; - const struct AddrinfoResult results[] = {{"127.0.0.1", .6666}, - {"127.0.0.1", TCP_SERVER_PORT}}; - - AddrinfoInjectSetResponse(0, 2, results); - - /* Use a check handle in order to close the transport in the same loop - * iteration where the second connection attempt occurs. */ - rv = uv_check_init(&f->loop, &check); - munit_assert_int(rv, ==, 0); - check.data = f; - CONNECT_REQ(2, "any-host", 0, RAFT_CANCELED); - /* Successfull DNS lookup will initiate async connect */ - LOOP_RUN(1); - uv_check_start(&check, checkCb); - LOOP_RUN(1); - LOOP_RUN_UNTIL(&_result.done); - CLOSE_WAIT; - return MUNIT_OK; -} diff --git a/test/raft/integration/test_uv_tcp_listen.c b/test/raft/integration/test_uv_tcp_listen.c index 747add3a6..b239cfa87 100644 --- a/test/raft/integration/test_uv_tcp_listen.c +++ b/test/raft/integration/test_uv_tcp_listen.c @@ -1,5 +1,6 @@ #include "../../../src/raft.h" #include "../../../src/raft/byte.h" +#include "../lib/addrinfo.h" #include "../lib/heap.h" #include "../lib/loop.h" #include "../lib/runner.h" @@ -94,6 +95,7 @@ static void *setUpDeps(const MunitParameter params[], MUNIT_UNUSED void *user_data) { struct fixture *f = munit_malloc(sizeof *f); + SET_UP_ADDRINFO; SET_UP_HEAP; SETUP_LOOP; SETUP_TCP; @@ -106,6 +108,7 @@ static void tearDownDeps(void *data) TEAR_DOWN_TCP; TEAR_DOWN_LOOP; TEAR_DOWN_HEAP; + TEAR_DOWN_ADDRINFO; free(f); } @@ -232,6 +235,83 @@ TEST(tcp_listen, invalidAddress, setUp, tearDown, 0, invalidTcpListenParams) return MUNIT_OK; } +/* Check success with addrinfo resolve to mutiple IP and first one is used to + * connect */ +TEST(tcp_listen, firstOfTwo, setUp, tearDown, 0, NULL) +{ + const struct AddrinfoResult results[] = {{"127.0.0.1", 9000}, + {"127.0.0.2", 9000}}; + struct fixture *f = data; + AddrinfoInjectSetResponse(0, 2, results); + LISTEN(0); + PEER_CONNECT; + PEER_HANDSHAKE; + ACCEPT; + return MUNIT_OK; +} + +/* Check success with addrinfo resolve to mutiple IP and second one is used to + * connect */ +TEST(tcp_listen, secondOfTwo, setUp, tearDown, 0, NULL) +{ + const struct AddrinfoResult results[] = {{"127.0.0.2", 9000}, + {"127.0.0.1", 9000}}; + struct fixture *f = data; + AddrinfoInjectSetResponse(0, 2, results); + + LISTEN(0); + PEER_CONNECT; + PEER_HANDSHAKE; + ACCEPT; + return MUNIT_OK; +} + +/* Simulate port already in use error by addrinfo response contain the same IP + * twice */ +TEST(tcp_listen, alreadyBound, setUp, tearDown, 0, NULL) +{ + /* We need to use the same endpoint three times as a simple duplicate will + * be skipped due to a glib strange behavior + * https://bugzilla.redhat.com/show_bug.cgi?id=496300 */ + const struct AddrinfoResult results[] = { + {"127.0.0.1", 9000}, {"127.0.0.1", 9000}, {"127.0.0.1", 9000}}; + struct fixture *f = data; + AddrinfoInjectSetResponse(0, 3, results); + LISTEN(RAFT_IOERR); + return MUNIT_OK; +} + +/* Error in bind first IP address */ +TEST(tcp_listen, cannotBindFirst, setUp, tearDown, 0, NULL) +{ + const struct AddrinfoResult results[] = {{"192.0.2.0", 9000}, + {"127.0.0.1", 9000}}; + struct fixture *f = data; + AddrinfoInjectSetResponse(0, 2, results); + LISTEN(RAFT_IOERR); + return MUNIT_OK; +} + +/* Error in bind of second IP address */ +TEST(tcp_listen, cannotBindSecond, setUp, tearDown, 0, NULL) +{ + const struct AddrinfoResult results[] = {{"127.0.0.1", 9000}, + {"192.0.2.0", 9000}}; + struct fixture *f = data; + AddrinfoInjectSetResponse(0, 2, results); + LISTEN(RAFT_IOERR); + return MUNIT_OK; +} + +/* Check error on general dns server failure */ +TEST(tcp_listen, resolveFailure, setUp, tearDown, 0, NULL) +{ + struct fixture *f = data; + AddrinfoInjectSetResponse(EAI_FAIL, 0, NULL); + LISTEN(RAFT_IOERR); + return MUNIT_OK; +} + /* The client sends us a bad protocol version */ TEST(tcp_listen, badProtocol, setUp, tearDown, 0, NULL) { diff --git a/test/raft/integration/test_uv_tcp_listen_addrinfo.c b/test/raft/integration/test_uv_tcp_listen_addrinfo.c deleted file mode 100644 index dc7123362..000000000 --- a/test/raft/integration/test_uv_tcp_listen_addrinfo.c +++ /dev/null @@ -1,268 +0,0 @@ -#include "../../../src/raft.h" -#include "../../../src/raft/byte.h" -#include "../lib/addrinfo.h" -#include "../lib/heap.h" -#include "../lib/loop.h" -#include "../lib/runner.h" -#include "../lib/tcp.h" - -/****************************************************************************** - * - * Fixture with a TCP-based raft_uv_transport. - * - *****************************************************************************/ - -struct fixture -{ - FIXTURE_HEAP; - FIXTURE_LOOP; - FIXTURE_TCP; - struct raft_uv_transport transport; - bool accepted; - bool closed; - struct - { - uint8_t buf[sizeof(uint64_t) + /* Protocol version */ - sizeof(uint64_t) + /* Server ID */ - sizeof(uint64_t) + /* Length of address */ - sizeof(uint64_t) * 2 /* Address */]; - size_t offset; - } handshake; -}; - -/****************************************************************************** - * - * Helper macros - * - *****************************************************************************/ - -#define PEER_ID 2 -#define PEER_ADDRESS "127.0.0.1:666" - -static void closeCb(struct raft_uv_transport *transport) -{ - struct fixture *f = transport->data; - f->closed = true; -} - -static void acceptCb(struct raft_uv_transport *t, - raft_id id, - const char *address, - struct uv_stream_s *stream) -{ - struct fixture *f = t->data; - munit_assert_int(id, ==, PEER_ID); - munit_assert_string_equal(address, PEER_ADDRESS); - f->accepted = true; - uv_close((struct uv_handle_s *)stream, (uv_close_cb)raft_free); -} - -#define INIT \ - do { \ - int _rv; \ - f->transport.version = 1; \ - _rv = raft_uv_tcp_init(&f->transport, &f->loop); \ - munit_assert_int(_rv, ==, 0); \ - const char *bind_addr = munit_parameters_get(params, "bind-address"); \ - if (bind_addr && strlen(bind_addr)) { \ - _rv = raft_uv_tcp_set_bind_address(&f->transport, bind_addr); \ - munit_assert_int(_rv, ==, 0); \ - } \ - const char *address = munit_parameters_get(params, "address"); \ - if (!address) { \ - address = "127.0.0.1:9000"; \ - } \ - _rv = f->transport.init(&f->transport, 1, address); \ - munit_assert_int(_rv, ==, 0); \ - f->transport.data = f; \ - f->closed = false; \ - } while (0) - -#define CLOSE \ - do { \ - f->transport.close(&f->transport, closeCb); \ - LOOP_RUN_UNTIL(&f->closed); \ - raft_uv_tcp_close(&f->transport); \ - } while (0) - -/****************************************************************************** - * - * Set up and tear down. - * - *****************************************************************************/ - -static void *setUpDeps(const MunitParameter params[], - MUNIT_UNUSED void *user_data) -{ - struct fixture *f = munit_malloc(sizeof *f); - SET_UP_ADDRINFO; - SET_UP_HEAP; - SETUP_LOOP; - SETUP_TCP; - return f; -} - -static void tearDownDeps(void *data) -{ - struct fixture *f = data; - TEAR_DOWN_TCP; - TEAR_DOWN_LOOP; - TEAR_DOWN_HEAP; - TEAR_DOWN_ADDRINFO; - free(f); -} - -static void *setUp(const MunitParameter params[], void *user_data) -{ - struct fixture *f = setUpDeps(params, user_data); - void *cursor; - /* test_tcp_listen(&f->tcp); */ - INIT; - f->accepted = false; - f->handshake.offset = 0; - - cursor = f->handshake.buf; - bytePut64(&cursor, 1); - bytePut64(&cursor, PEER_ID); - bytePut64(&cursor, 16); - strcpy(cursor, PEER_ADDRESS); - - return f; -} - -static void tearDown(void *data) -{ - struct fixture *f = data; - CLOSE; - tearDownDeps(f); -} - -/****************************************************************************** - * - * Helper macros - * - *****************************************************************************/ - -#define LISTEN(EXPECTED_RV) \ - do { \ - int rv; \ - rv = f->transport.listen(&f->transport, acceptCb); \ - munit_assert_int(rv, ==, EXPECTED_RV); \ - } while (false) - -/* Connect to the listening socket of the transport, creating a new connection - * that is waiting to be accepted. */ -#define PEER_CONNECT TCP_CLIENT_CONNECT(9000) - -/* Make the peer close the connection. */ -#define PEER_CLOSE TCP_CLIENT_CLOSE - -/* Make the connected client send handshake data. */ -#define PEER_HANDSHAKE \ - do { \ - size_t n = sizeof f->handshake.buf; \ - TCP_CLIENT_SEND(f->handshake.buf, n); \ - } while (0) - -/* Make the connected client send partial handshake data: only N bytes will be - * sent, starting from the offset of the last call. */ -#define PEER_HANDSHAKE_PARTIAL(N) \ - do { \ - TCP_CLIENT_SEND(f->handshake.buf + f->handshake.offset, N); \ - } while (0) - -/* After a PEER_CONNECT() call, spin the event loop until the connected - * callback of the listening TCP handle gets called. */ -#define LOOP_RUN_UNTIL_CONNECTED LOOP_RUN(1); - -/* After a PEER_HANDSHAKE_PARTIAL() call, spin the event loop until the read - * callback gets called. */ -#define LOOP_RUN_UNTIL_READ LOOP_RUN(1); - -/* Spin the event loop until the accept callback gets eventually invoked. */ -#define ACCEPT LOOP_RUN_UNTIL(&f->accepted); - -/****************************************************************************** - * - * Success scenarios. - * - *****************************************************************************/ - -SUITE(tcp_listen) - -/* Check success with addrinfo resolve to mutiple IP and first one is used to - * connect */ -TEST(tcp_listen, firstOfTwo, setUp, tearDown, 0, NULL) -{ - const struct AddrinfoResult results[] = {{"127.0.0.1", 9000}, - {"127.0.0.2", 9000}}; - struct fixture *f = data; - AddrinfoInjectSetResponse(0, 2, results); - LISTEN(0); - PEER_CONNECT; - PEER_HANDSHAKE; - ACCEPT; - return MUNIT_OK; -} - -/* Check success with addrinfo resolve to mutiple IP and second one is used to - * connect */ -TEST(tcp_listen, secondOfTwo, setUp, tearDown, 0, NULL) -{ - const struct AddrinfoResult results[] = {{"127.0.0.2", 9000}, - {"127.0.0.1", 9000}}; - struct fixture *f = data; - AddrinfoInjectSetResponse(0, 2, results); - - LISTEN(0); - PEER_CONNECT; - PEER_HANDSHAKE; - ACCEPT; - return MUNIT_OK; -} - -/* Simulate port already in use error by addrinfo response contain the same IP - * twice */ -TEST(tcp_listen, alreadyBound, setUp, tearDown, 0, NULL) -{ - /* We need to use the same endpoint three times as a simple duplicate will - * be skipped due to a glib strange behavior - * https://bugzilla.redhat.com/show_bug.cgi?id=496300 */ - const struct AddrinfoResult results[] = { - {"127.0.0.1", 9000}, {"127.0.0.1", 9000}, {"127.0.0.1", 9000}}; - struct fixture *f = data; - AddrinfoInjectSetResponse(0, 3, results); - LISTEN(RAFT_IOERR); - return MUNIT_OK; -} - -/* Error in bind first IP address */ -TEST(tcp_listen, cannotBindFirst, setUp, tearDown, 0, NULL) -{ - const struct AddrinfoResult results[] = {{"192.0.2.0", 9000}, - {"127.0.0.1", 9000}}; - struct fixture *f = data; - AddrinfoInjectSetResponse(0, 2, results); - LISTEN(RAFT_IOERR); - return MUNIT_OK; -} - -/* Error in bind of second IP address */ -TEST(tcp_listen, cannotBindSecond, setUp, tearDown, 0, NULL) -{ - const struct AddrinfoResult results[] = {{"127.0.0.1", 9000}, - {"192.0.2.0", 9000}}; - struct fixture *f = data; - AddrinfoInjectSetResponse(0, 2, results); - LISTEN(RAFT_IOERR); - return MUNIT_OK; -} - -/* Check error on general dns server failure */ -TEST(tcp_listen, resolveFailure, setUp, tearDown, 0, NULL) -{ - struct fixture *f = data; - AddrinfoInjectSetResponse(EAI_FAIL, 0, NULL); - LISTEN(RAFT_IOERR); - return MUNIT_OK; -} From 7a07c24e7cf805f781f2e99f8b9e0d5f938a08e5 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 29 Aug 2024 16:42:25 -0400 Subject: [PATCH 14/35] fixup static build configuration Signed-off-by: Cole Miller --- Makefile.am | 26 +++++++++++++++----------- configure.ac | 5 ++--- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/Makefile.am b/Makefile.am index f0d131e3d..4ffed0fdb 100644 --- a/Makefile.am +++ b/Makefile.am @@ -2,7 +2,16 @@ ACLOCAL_AMFLAGS = -I m4 AM_CFLAGS += $(CODE_COVERAGE_CFLAGS) AM_CFLAGS += $(SQLITE_CFLAGS) $(UV_CFLAGS) $(PTHREAD_CFLAGS) -AM_LDFLAGS = $(UV_LIBS) $(PTHREAD_LIBS) + +if WITH_STATIC_DEPS +AM_CFLAGS += -DDQLITE_STATIC_LIBC +static = -all-static +else +static = +endif + +AM_LDFLAGS = $(static) +AM_LDFLAGS += $(UV_LIBS) $(PTHREAD_LIBS) if DQLITE_NEXT_ENABLED AM_CFLAGS += -DDQLITE_NEXT @@ -28,11 +37,6 @@ AM_CFLAGS += -DDQLITE_ASSERT_WITH_BACKTRACE -DRAFT_ASSERT_WITH_BACKTRACE AM_LDFLAGS += -lbacktrace endif -if WITH_STATIC_DEPS -AM_CFLAGS += -DDQLITE_STATIC_LIBC -AM_LDFLAGS += -all-static -endif - include_HEADERS = include/dqlite.h basic_dqlite_sources = \ @@ -221,7 +225,7 @@ libtest_la_SOURCES += \ test/raft/lib/loop.c libraft_la_CFLAGS = $(AM_CFLAGS) -libraft_la_LDFLAGS = $(UV_LIBS) +libraft_la_LDFLAGS = $(static) $(UV_LIBS) raft_core_unit_test_SOURCES = \ $(libraft_la_SOURCES) \ @@ -265,7 +269,7 @@ raft_core_integration_test_SOURCES = \ test/raft/integration/test_transfer.c \ test/raft/integration/test_voter_contacts.c raft_core_integration_test_CFLAGS = $(AM_CFLAGS) -Wno-conversion -raft_core_integration_test_LDFLAGS = -no-install +raft_core_integration_test_LDFLAGS = $(static) -no-install raft_core_integration_test_LDADD = libtest.la libraft.la raft_core_fuzzy_test_SOURCES = \ @@ -278,7 +282,7 @@ raft_core_fuzzy_test_SOURCES = \ test/raft/fuzzy/test_membership.c \ test/raft/fuzzy/test_replication.c raft_core_fuzzy_test_CFLAGS = $(AM_CFLAGS) -Wno-conversion -raft_core_fuzzy_test_LDFLAGS = -no-install +raft_core_fuzzy_test_LDFLAGS = $(static) -no-install raft_core_fuzzy_test_LDADD = libtest.la libraft.la raft_uv_unit_test_SOURCES = \ @@ -320,14 +324,14 @@ raft_uv_integration_test_SOURCES = \ test/raft/integration/test_uv_truncate_snapshot.c \ test/raft/integration/test_uv_work.c raft_uv_integration_test_CFLAGS = $(AM_CFLAGS) -Wno-type-limits -Wno-conversion -raft_uv_integration_test_LDFLAGS = -no-install +raft_uv_integration_test_LDFLAGS = $(static) -no-install raft_uv_integration_test_LDADD = libtest.la $(UV_LIBS) if LZ4_AVAILABLE libdqlite_la_CFLAGS += -DLZ4_AVAILABLE $(LZ4_CFLAGS) libdqlite_la_LDFLAGS += $(LZ4_LIBS) raft_core_unit_test_CFLAGS += -DLZ4_AVAILABLE $(LZ4_CFLAGS) -raft_core_unit_test_LDFLAGS = $(LZ4_LIBS) +raft_core_unit_test_LDFLAGS = $(static) $(LZ4_LIBS) libraft_la_CFLAGS += -DLZ4_AVAILABLE $(LZ4_CFLAGS) libraft_la_LDFLAGS += $(LZ4_LIBS) raft_uv_integration_test_CFLAGS += -DLZ4_AVAILABLE diff --git a/configure.ac b/configure.ac index fbbe048de..381b47617 100644 --- a/configure.ac +++ b/configure.ac @@ -44,9 +44,8 @@ AS_IF([test "x$enable_build_raft" != "xyes" -a "x$enable_dqlite_next" = "xyes"], AC_ARG_WITH(static-deps, AS_HELP_STRING([--with-static-deps[=ARG]], - [skip building a shared library and link test binaries statically]) - [], [with_static_deps=no]) -AM_CONDITIONAL(WITH_STATIC_DEPS, test "x$with_static_deps" != "xno") + [skip building a shared library and link test binaries statically])) +AM_CONDITIONAL(WITH_STATIC_DEPS, test "x$with_static_deps" = "xyes") # Whether to enable code coverage. AX_CODE_COVERAGE From 5e9c7e7c66166fb926859a279f1c2db5169ba6a0 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 29 Aug 2024 16:58:09 -0400 Subject: [PATCH 15/35] Skip addrinfo tests when libc is statically linked Signed-off-by: Cole Miller --- Makefile.am | 5 +++- test/raft/integration/test_uv_tcp_connect.c | 6 ++-- test/raft/integration/test_uv_tcp_listen.c | 12 ++++---- test/raft/lib/addrinfo.h | 32 +++++++++++++++++++-- 4 files changed, 42 insertions(+), 13 deletions(-) diff --git a/Makefile.am b/Makefile.am index 4ffed0fdb..0a2d0d7cb 100644 --- a/Makefile.am +++ b/Makefile.am @@ -212,7 +212,6 @@ check_PROGRAMS += \ raft-core-fuzzy-test libtest_la_SOURCES += \ - test/raft/lib/addrinfo.c \ test/raft/lib/fault.c \ test/raft/lib/fsm.c \ test/raft/lib/heap.c \ @@ -224,6 +223,10 @@ libtest_la_SOURCES += \ test/raft/lib/tcp.c \ test/raft/lib/loop.c +if !WITH_STATIC_DEPS +libtest_la_SOURCES += test/raft/lib/addrinfo.c +endif + libraft_la_CFLAGS = $(AM_CFLAGS) libraft_la_LDFLAGS = $(static) $(UV_LIBS) diff --git a/test/raft/integration/test_uv_tcp_connect.c b/test/raft/integration/test_uv_tcp_connect.c index 7efc68c60..9fc578903 100644 --- a/test/raft/integration/test_uv_tcp_connect.c +++ b/test/raft/integration/test_uv_tcp_connect.c @@ -192,7 +192,7 @@ TEST(tcp_connect, connectByName, setUp, tearDown, 0, NULL) } /* Successfully connect to the peer by first IP */ -TEST(tcp_connect, firstIP, setUp, tearDown, 0, NULL) +ADDRINFO_TEST(tcp_connect, firstIP, setUp, tearDown, 0, NULL) { struct fixture *f = data; const struct AddrinfoResult results[] = {{"127.0.0.1", TCP_SERVER_PORT}, @@ -203,7 +203,7 @@ TEST(tcp_connect, firstIP, setUp, tearDown, 0, NULL) } /* Successfully connect to the peer by second IP */ -TEST(tcp_connect, secondIP, setUp, tearDown, 0, NULL) +ADDRINFO_TEST(tcp_connect, secondIP, setUp, tearDown, 0, NULL) { struct fixture *f = data; const struct AddrinfoResult results[] = {{"127.0.0.1", .6666}, @@ -332,7 +332,7 @@ TEST(tcp_connect, closeDuringConnectAbort, setUp, tearDownDeps, 0, NULL) /* The transport gets closed right after the first connection attempt failed, * while doing a second connection attempt. */ -TEST(tcp_connect, closeDuringSecondConnect, setUp, tearDownDeps, 0, NULL) +ADDRINFO_TEST(tcp_connect, closeDuringSecondConnect, setUp, tearDownDeps, 0, NULL) { struct fixture *f = data; struct uv_check_s check; diff --git a/test/raft/integration/test_uv_tcp_listen.c b/test/raft/integration/test_uv_tcp_listen.c index b239cfa87..e4ec757bc 100644 --- a/test/raft/integration/test_uv_tcp_listen.c +++ b/test/raft/integration/test_uv_tcp_listen.c @@ -237,7 +237,7 @@ TEST(tcp_listen, invalidAddress, setUp, tearDown, 0, invalidTcpListenParams) /* Check success with addrinfo resolve to mutiple IP and first one is used to * connect */ -TEST(tcp_listen, firstOfTwo, setUp, tearDown, 0, NULL) +ADDRINFO_TEST(tcp_listen, firstOfTwo, setUp, tearDown, 0, NULL) { const struct AddrinfoResult results[] = {{"127.0.0.1", 9000}, {"127.0.0.2", 9000}}; @@ -252,7 +252,7 @@ TEST(tcp_listen, firstOfTwo, setUp, tearDown, 0, NULL) /* Check success with addrinfo resolve to mutiple IP and second one is used to * connect */ -TEST(tcp_listen, secondOfTwo, setUp, tearDown, 0, NULL) +ADDRINFO_TEST(tcp_listen, secondOfTwo, setUp, tearDown, 0, NULL) { const struct AddrinfoResult results[] = {{"127.0.0.2", 9000}, {"127.0.0.1", 9000}}; @@ -268,7 +268,7 @@ TEST(tcp_listen, secondOfTwo, setUp, tearDown, 0, NULL) /* Simulate port already in use error by addrinfo response contain the same IP * twice */ -TEST(tcp_listen, alreadyBound, setUp, tearDown, 0, NULL) +ADDRINFO_TEST(tcp_listen, alreadyBound, setUp, tearDown, 0, NULL) { /* We need to use the same endpoint three times as a simple duplicate will * be skipped due to a glib strange behavior @@ -282,7 +282,7 @@ TEST(tcp_listen, alreadyBound, setUp, tearDown, 0, NULL) } /* Error in bind first IP address */ -TEST(tcp_listen, cannotBindFirst, setUp, tearDown, 0, NULL) +ADDRINFO_TEST(tcp_listen, cannotBindFirst, setUp, tearDown, 0, NULL) { const struct AddrinfoResult results[] = {{"192.0.2.0", 9000}, {"127.0.0.1", 9000}}; @@ -293,7 +293,7 @@ TEST(tcp_listen, cannotBindFirst, setUp, tearDown, 0, NULL) } /* Error in bind of second IP address */ -TEST(tcp_listen, cannotBindSecond, setUp, tearDown, 0, NULL) +ADDRINFO_TEST(tcp_listen, cannotBindSecond, setUp, tearDown, 0, NULL) { const struct AddrinfoResult results[] = {{"127.0.0.1", 9000}, {"192.0.2.0", 9000}}; @@ -304,7 +304,7 @@ TEST(tcp_listen, cannotBindSecond, setUp, tearDown, 0, NULL) } /* Check error on general dns server failure */ -TEST(tcp_listen, resolveFailure, setUp, tearDown, 0, NULL) +ADDRINFO_TEST(tcp_listen, resolveFailure, setUp, tearDown, 0, NULL) { struct fixture *f = data; AddrinfoInjectSetResponse(EAI_FAIL, 0, NULL); diff --git a/test/raft/lib/addrinfo.h b/test/raft/lib/addrinfo.h index cc29d5864..7921412a9 100644 --- a/test/raft/lib/addrinfo.h +++ b/test/raft/lib/addrinfo.h @@ -16,15 +16,39 @@ #include "munit.h" -#define SET_UP_ADDRINFO AddrinfoInjectSetUp(params) -#define TEAR_DOWN_ADDRINFO AddrinfoInjectTearDown() - typedef struct AddrinfoResult { const char *ip; const int port; } AddrinfoResult_t; +#ifdef DQLITE_STATIC_LIBC + +#define ADDRINFO_TEST(S, C, SETUP, TEAR_DOWN, OPTIONS, PARAMS) \ + TEST(S, C, SETUP, TEAR_DOWN, OPTIONS, PARAMS) \ + { \ + return MUNIT_SKIP; \ + } \ + static MUNIT_UNUSED MunitResult test_unused_##S##_##C( \ + MUNIT_UNUSED const MunitParameter params[], MUNIT_UNUSED void *data) + +#define SET_UP_ADDRINFO +#define TEAR_DOWN_ADDRINFO +#define AddrinfoInjectSetResponse(a, b, c) \ + do { \ + (void)a; \ + (void)b; \ + (void)c; \ + } while (0) + +#else /* ifndef DQLITE_STATIC_LIBC */ + +#define ADDRINFO_TEST(S, C, SETUP, TEAR_DOWN, OPTIONS, PARAMS) \ + TEST(S, C, SETUP, TEAR_DOWN, OPTIONS, PARAMS) + +#define SET_UP_ADDRINFO AddrinfoInjectSetUp(params) +#define TEAR_DOWN_ADDRINFO AddrinfoInjectTearDown() + void AddrinfoInjectSetResponse(int rv, int num_results, const struct AddrinfoResult *results); @@ -32,4 +56,6 @@ void AddrinfoInjectSetResponse(int rv, void AddrinfoInjectSetUp(const MunitParameter params[]); void AddrinfoInjectTearDown(void); +#endif /* ifdef DQLITE_STATIC_LIBC ... else */ + #endif // #ifndef TEST_ADDRINFO_H From 25800724b46b0e5ca0ea5e486bd1af2e2d698e5d Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 29 Aug 2024 18:32:57 -0400 Subject: [PATCH 16/35] Use latest versions of dependencies in build-static.sh Signed-off-by: Cole Miller --- .github/workflows/static.yml | 3 ++- contrib/build-static.sh | 40 +++++++++++++++++++++--------------- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/.github/workflows/static.yml b/.github/workflows/static.yml index d35b15899..10a0a32e7 100644 --- a/.github/workflows/static.yml +++ b/.github/workflows/static.yml @@ -16,6 +16,7 @@ jobs: sudo apt install -y build-essential automake libtool gettext autopoint tclsh tcl libsqlite3-dev pkg-config git - name: Build and test + env: + LIBDQLITE_TRACE: 1 run: | - export LIBDQLITE_TRACE=1 contrib/build-static.sh diff --git a/contrib/build-static.sh b/contrib/build-static.sh index ae3f33e23..6a8b73f91 100755 --- a/contrib/build-static.sh +++ b/contrib/build-static.sh @@ -8,15 +8,9 @@ REPO_LIBNSL="https://github.com/thkukuk/libnsl.git" REPO_LIBUV="https://github.com/libuv/libuv.git" REPO_LIBLZ4="https://github.com/lz4/lz4.git" REPO_SQLITE="https://github.com/sqlite/sqlite.git" -REPO_DQLITE="https://github.com/cole-miller/dqlite.git" -TAG_MUSL="v1.2.4" -TAG_LIBTIRPC="upstream/1.3.3" -TAG_LIBNSL="v2.0.1" -TAG_LIBUV="v1.48.0" -TAG_LIBLZ4="v1.9.4" -TAG_SQLITE="version-3.45.1" -TAG_DQLITE="musl-ci" +TAG_MUSL="${TAG_MUSL:-v1.2.4}" +DQLITE_PATH="${DQLITE_PATH:-$DIR/..}" BUILD_DIR="${DIR}/build" INSTALL_DIR="${DIR}/prefix" @@ -32,6 +26,21 @@ if [ "${MACHINE_TYPE}" = "ppc64le" ]; then fi export PKG_CONFIG_PATH="${INSTALL_DIR}/lib/pkgconfig" +clone-latest-tag() { + name="$1" + repo="$2" + tagpattern="${3:-.*}" + mkdir "${name}" + pushd "${name}" + git init + git remote add upstream "${repo}" + git fetch upstream 'refs/tags/*:refs/tags/*' + tag="$(git tag | grep "${tagpattern}" | sort -V -r | head -n1)" + echo "Selected $name tag ${tag}" + git checkout "${tag}" + popd +} + # build musl if [ ! -f "${INSTALL_DIR}/musl/bin/musl-gcc" ]; then ( @@ -61,7 +70,7 @@ if [ ! -f "${BUILD_DIR}/libtirpc/src/libtirpc.la" ]; then ( cd "${BUILD_DIR}" rm -rf libtirpc - git clone "${REPO_LIBTIRPC}" --depth 1 --branch "${TAG_LIBTIRPC}" + clone-latest-tag libtirpc "${REPO_LIBTIRPC}" upstream cd libtirpc chmod +x autogen.sh ./autogen.sh @@ -75,7 +84,7 @@ if [ ! -f "${BUILD_DIR}/libnsl/src/libnsl.la" ]; then ( cd "${BUILD_DIR}" rm -rf libnsl - git clone "${REPO_LIBNSL}" --depth 1 --branch "${TAG_LIBNSL}" + clone-latest-tag libnsl "${REPO_LIBNSL}" cd libnsl ./autogen.sh autoreconf -i @@ -90,7 +99,7 @@ if [ ! -f "${BUILD_DIR}/libuv/libuv.la" ]; then ( cd "${BUILD_DIR}" rm -rf libuv - git clone "${REPO_LIBUV}" --depth 1 --branch "${TAG_LIBUV}" + clone-latest-tag libuv "${REPO_LIBUV}" cd libuv ./autogen.sh ./configure --disable-shared --prefix="${INSTALL_DIR}" @@ -103,7 +112,7 @@ if [ ! -f "${BUILD_DIR}/lz4/lib/liblz4.a" ] || [ ! -f "${BUILD_DIR}/lz4/lib/libl ( cd "${BUILD_DIR}" rm -rf lz4 - git clone "${REPO_LIBLZ4}" --depth 1 --branch "${TAG_LIBLZ4}" + clone-latest-tag lz4 "${REPO_LIBLZ4}" cd lz4 make install -j PREFIX="${INSTALL_DIR}" BUILD_SHARED=no ) @@ -114,7 +123,7 @@ if [ ! -f "${BUILD_DIR}/sqlite/libsqlite3.la" ]; then ( cd "${BUILD_DIR}" rm -rf sqlite - git clone "${REPO_SQLITE}" --depth 1 --branch "${TAG_SQLITE}" + clone-latest-tag sqlite "${REPO_SQLITE}" cd sqlite ./configure --disable-shared --disable-readline --prefix="${INSTALL_DIR}" \ CFLAGS="${CFLAGS} -DSQLITE_ENABLE_DBSTAT_VTAB=1" @@ -125,10 +134,7 @@ fi # build dqlite if [ ! -f "${BUILD_DIR}/dqlite/libdqlite.la" ]; then ( - cd "${BUILD_DIR}" - rm -rf dqlite - git clone "${REPO_DQLITE}" --depth 1 --branch "${TAG_DQLITE}" - cd dqlite + cd "${DQLITE_PATH}" autoreconf -i ./configure --enable-build-raft --with-static-deps --prefix="${INSTALL_DIR}" make -j check-norun From 6557dd732511f58349746968f9e8c37df90846c6 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 29 Aug 2024 18:40:18 -0400 Subject: [PATCH 17/35] Fix Signed-off-by: Cole Miller --- Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.am b/Makefile.am index 0a2d0d7cb..beb79c7ed 100644 --- a/Makefile.am +++ b/Makefile.am @@ -365,7 +365,7 @@ TESTS = $(check_PROGRAMS) check-norun: $(check_PROGRAMS) -print-test-binaries: +print-test-programs: @echo '$(check_PROGRAMS)' if CODE_COVERAGE_ENABLED From 788d99a85360e427040150d7da5757ea57ef8d8d Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 29 Aug 2024 19:57:05 -0400 Subject: [PATCH 18/35] Remove `-fexceptions` and `-fno-exceptions` Signed-off-by: Cole Miller --- configure.ac | 2 -- 1 file changed, 2 deletions(-) diff --git a/configure.ac b/configure.ac index 381b47617..021ea7b0d 100644 --- a/configure.ac +++ b/configure.ac @@ -100,10 +100,8 @@ CC_CHECK_FLAGS_APPEND([AM_CFLAGS],[CFLAGS],[ \ -pipe \ -fno-strict-aliasing \ -fdiagnostics-color \ - -fexceptions \ -fstack-clash-protection \ -fstack-protector-strong \ - -fno-exceptions \ -fdiagnostics-show-option \ -Wall \ -Wextra \ From 1dc45ba120efefe9b31dd79e69a063d31186e2f0 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 29 Aug 2024 20:39:25 -0400 Subject: [PATCH 19/35] Fix the build when liblz4 is missing and test it Signed-off-by: Cole Miller --- .github/workflows/nolz4.yaml | 39 ++++++++++++++++++++++++++++++++++++ configure.ac | 2 +- 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/nolz4.yaml diff --git a/.github/workflows/nolz4.yaml b/.github/workflows/nolz4.yaml new file mode 100644 index 000000000..40f8f2f71 --- /dev/null +++ b/.github/workflows/nolz4.yaml @@ -0,0 +1,39 @@ +name: CI Tests (no liblz4) + +on: + - push + - pull_request + +jobs: + build: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v4 + + - name: Set up dependencies + run: | + sudo apt update + sudo apt install -y libsqlite3-dev libuv1-dev + sudo apt remove -y liblz4-dev + + - name: Build dqlite (liblz4 not present) + run: | + autoreconf -i + ./configure --enable-build-raft + make -j$(nproc) + make clean + + - name: Build dqlite (liblz4 requested and not present) + run: | + autoreconf -i + ! ./configure --enable-build-raft --with-lz4 + - name: Install liblz4 + run: | + sudo apt install liblz4-dev + + - name: Build dqlite (liblz4 present but ignored) + run: | + ./configure --enable-build-raft --without-lz4 + make -j$(nproc) + ! ldd .libs/libdqlite.so | grep lz4 diff --git a/configure.ac b/configure.ac index 6bc4afebc..1d7839441 100644 --- a/configure.ac +++ b/configure.ac @@ -70,7 +70,7 @@ AS_IF([test "x$enable_build_raft" = "xyes"], [AS_IF([test "x$with_lz4" != "xno"], [PKG_CHECK_MODULES(LZ4, [liblz4 >= 1.7.1], [have_lz4=yes], [have_lz4=no])], [have_lz4=no]) - AS_IF([test "x$with_lz4" != "xno" -a "x$have_lz4" = "xno"], + AS_IF([test "x$with_lz4" = "xyes" -a "x$have_lz4" = "xno"], [AC_MSG_ERROR([liblz4 required but not found])], [])], # Not building raft From 44c2889b9bc313b5472122ffdb22194cb56eb986 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 29 Aug 2024 21:34:51 -0400 Subject: [PATCH 20/35] Grab stack traces when unit test hangs Signed-off-by: Cole Miller --- .github/workflows/build-and-test.yml | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 31da63589..91397bb27 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -48,20 +48,18 @@ jobs: CC: ${{ matrix.compiler }} LIBDQLITE_TRACE: 1 run: | - for bin in unit-test integration-test \ - raft-core-fuzzy-test raft-core-integration-test \ - raft-core-unit-test raft-uv-integration-test \ - raft-uv-unit-test - do ./$bin || touch any-failed - done 2>&1 | tee -a test-suite.log - test '!' -e any-failed + tests="integration-test \ + raft-core-fuzzy-test \ + raft-core-integration-test \ + raft-core-unit-test \ + raft-uv-integration-test \ + raft-uv-unit-test" + make check TESTS="$tests" - - name: OnTestFailure - if: ${{ failure() }} - uses: actions/upload-artifact@v3 - with: - name: test-suite.log - path: test-suite.log + ./unit-test --no-fork & + pid=$! + bash -c "sleep 10m; sudo gdb -p $pid -batch -ex 'thread apply all bt' -ex quit; false" & + wait -n - name: Coverage env: From b801c9c4d7a05aa1ee3a56118c8bbd644c839f2c Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Fri, 30 Aug 2024 00:39:46 -0400 Subject: [PATCH 21/35] Add SQLite fixture in two places where it's missing Signed-off-by: Cole Miller --- test/unit/lib/test_registry.c | 5 +++++ test/unit/test_vfs2.c | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/test/unit/lib/test_registry.c b/test/unit/lib/test_registry.c index af2bd6625..48c71a039 100644 --- a/test/unit/lib/test_registry.c +++ b/test/unit/lib/test_registry.c @@ -3,6 +3,7 @@ #include "../../../src/lib/registry.h" #include "../../lib/runner.h" +#include "../../lib/sqlite.h" TEST_MODULE(lib_registry); @@ -45,6 +46,8 @@ static void *setup(const MunitParameter params[], void *user_data) (void)params; (void)user_data; + SETUP_SQLITE; + registry = (struct test_registry *)munit_malloc(sizeof(*registry)); test_registry_init(registry); @@ -58,6 +61,8 @@ static void tear_down(void *data) test_registry_close(registry); free(registry); + + TEAR_DOWN_SQLITE; } TEST_SUITE(add); diff --git a/test/unit/test_vfs2.c b/test/unit/test_vfs2.c index 635d7ce7f..62c332de9 100644 --- a/test/unit/test_vfs2.c +++ b/test/unit/test_vfs2.c @@ -4,6 +4,7 @@ #include "../../src/lib/byte.h" #include "../lib/fs.h" #include "../lib/runner.h" +#include "../lib/sqlite.h" #include @@ -29,6 +30,9 @@ static void *set_up(const MunitParameter params[], void *user_data) (void)params; (void)user_data; struct fixture *f = munit_malloc(sizeof(*f)); + + SETUP_SQLITE; + f->dir = test_dir_setup(); f->vfs = vfs2_make(sqlite3_vfs_find("unix"), "dqlite-vfs2"); munit_assert_ptr_not_null(f->vfs); @@ -43,6 +47,8 @@ static void tear_down(void *data) vfs2_destroy(f->vfs); test_dir_tear_down(f->dir); free(f); + + TEAR_DOWN_SQLITE; } static void prepare_wals(const char *dbname, From 77d99b58b241b77bb78fc123a758dbb0fe9d9ffb Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Fri, 30 Aug 2024 01:06:36 -0400 Subject: [PATCH 22/35] Revert "test: Don't call sqlite3_config to replace allocator after setup" This reverts commit 8737e068608479aa2e3842cf83aee8e452c985c9. --- test/lib/heap.c | 67 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 47 insertions(+), 20 deletions(-) diff --git a/test/lib/heap.c b/test/lib/heap.c index 4db0cf2c8..dba01591a 100644 --- a/test/lib/heap.c +++ b/test/lib/heap.c @@ -1,4 +1,3 @@ -#include #include #include "fault.h" @@ -98,6 +97,15 @@ static void mem_wrap(sqlite3_mem_methods *m, sqlite3_mem_methods *wrap) wrap->pAppData = &memFault; } +/* Unwrap the given faulty memory management instance returning the original + * one. */ +static void mem_unwrap(sqlite3_mem_methods *wrap, sqlite3_mem_methods *m) +{ + (void)wrap; + + *m = memFault.m; +} + /* Get the current number of outstanding malloc()'s without a matching free() * and the total number of used memory. */ static void mem_stats(int *malloc_count, int *memory_used) @@ -118,12 +126,22 @@ static void mem_stats(int *malloc_count, int *memory_used) } } -static void replace_sqlite_heap(void) +/* Ensure we're starting from a clean memory state with no allocations and + * optionally inject malloc failures. */ +void test_heap_setup(const MunitParameter params[], void *user_data) { + int malloc_count; + int memory_used; + const char *fault_delay; + const char *fault_repeat; sqlite3_mem_methods mem; sqlite3_mem_methods mem_fault; int rc; + (void)params; + (void)user_data; + + /* Install the faulty malloc implementation */ rc = sqlite3_config(SQLITE_CONFIG_GETMALLOC, &mem); if (rc != SQLITE_OK) { munit_errorf("can't get default mem: %s", sqlite3_errstr(rc)); @@ -135,24 +153,6 @@ static void replace_sqlite_heap(void) if (rc != SQLITE_OK) { munit_errorf("can't set faulty mem: %s", sqlite3_errstr(rc)); } -} - -/* Ensure we're starting from a clean memory state with no allocations and - * optionally inject malloc failures. */ -void test_heap_setup(const MunitParameter params[], void *user_data) -{ - int malloc_count; - int memory_used; - const char *fault_delay; - const char *fault_repeat; - int rc; - - (void)params; - (void)user_data; - - static pthread_once_t once = PTHREAD_ONCE_INIT; - rc = pthread_once(&once, replace_sqlite_heap); - munit_assert_int(rc, ==, 0); /* Check that memory is clean. */ mem_stats(&malloc_count, &memory_used); @@ -177,7 +177,34 @@ void test_heap_setup(const MunitParameter params[], void *user_data) /* Ensure we're starting leaving a clean memory behind. */ void test_heap_tear_down(void *data) { + sqlite3_mem_methods mem; + sqlite3_mem_methods mem_fault; + int rc; + (void)data; + + int malloc_count; + int memory_used; + + mem_stats(&malloc_count, &memory_used); + if (malloc_count > 0 || memory_used > 0) { + /* munit_errorf( + "teardown memory:\n bytes: %11d\n allocations: %5d\n", + memory_used, malloc_count); */ + } + + /* Restore default memory management. */ + rc = sqlite3_config(SQLITE_CONFIG_GETMALLOC, &mem_fault); + if (rc != SQLITE_OK) { + munit_errorf("can't get faulty mem: %s", sqlite3_errstr(rc)); + } + + mem_unwrap(&mem_fault, &mem); + + rc = sqlite3_config(SQLITE_CONFIG_MALLOC, &mem); + if (rc != SQLITE_OK) { + munit_errorf("can't reset default mem: %s", sqlite3_errstr(rc)); + } } void test_heap_fault_config(int delay, int repeat) From 494d6e711146117bb63c5f8fee89cf95998245f1 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Fri, 30 Aug 2024 07:55:45 -0400 Subject: [PATCH 23/35] Address comments from review Signed-off-by: Cole Miller --- .github/workflows/build-and-test.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 91397bb27..83bac5b68 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -28,6 +28,8 @@ jobs: run: | sudo apt update sudo apt install -y lcov libsqlite3-dev liblz4-dev libuv1-dev + # TODO: remove once the mysterious hang is fixed + sudo apt install -y gdb - name: Build dqlite env: @@ -48,6 +50,7 @@ jobs: CC: ${{ matrix.compiler }} LIBDQLITE_TRACE: 1 run: | + # TODO: return to just `make check` once the mysterious hang is fixed tests="integration-test \ raft-core-fuzzy-test \ raft-core-integration-test \ From 63367d704597abe44888a644ad0cd78175e19e77 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Sun, 1 Sep 2024 16:11:55 -0400 Subject: [PATCH 24/35] Improve and document addrinfo fixture for WITH_STATIC_DEPS Signed-off-by: Cole Miller --- Makefile.am | 12 +++++----- test/raft/lib/addrinfo.c | 24 ++++++++++++++++++++ test/raft/lib/addrinfo.h | 49 ++++++++++++++++++---------------------- 3 files changed, 52 insertions(+), 33 deletions(-) diff --git a/Makefile.am b/Makefile.am index beb79c7ed..96fbeb963 100644 --- a/Makefile.am +++ b/Makefile.am @@ -212,6 +212,7 @@ check_PROGRAMS += \ raft-core-fuzzy-test libtest_la_SOURCES += \ + test/raft/lib/addrinfo.c \ test/raft/lib/fault.c \ test/raft/lib/fsm.c \ test/raft/lib/heap.c \ @@ -223,10 +224,6 @@ libtest_la_SOURCES += \ test/raft/lib/tcp.c \ test/raft/lib/loop.c -if !WITH_STATIC_DEPS -libtest_la_SOURCES += test/raft/lib/addrinfo.c -endif - libraft_la_CFLAGS = $(AM_CFLAGS) libraft_la_LDFLAGS = $(static) $(UV_LIBS) @@ -363,10 +360,13 @@ endif TESTS = $(check_PROGRAMS) -check-norun: $(check_PROGRAMS) +check-norun: $(TESTS) +# Sometimes we want to run the test binaries manually. This target allows us to +# get the list of binaries out of the build system for that use-case, +# maintaining a single source of truth. print-test-programs: - @echo '$(check_PROGRAMS)' + @echo '$(TESTS)' if CODE_COVERAGE_ENABLED diff --git a/test/raft/lib/addrinfo.c b/test/raft/lib/addrinfo.c index 532ddab5f..0cfad2a93 100644 --- a/test/raft/lib/addrinfo.c +++ b/test/raft/lib/addrinfo.c @@ -7,6 +7,28 @@ #include #include +#ifdef WITH_STATIC_DEPS + +void AddrinfoInjectSetResponse(int rv, + int num_results, + const struct AddrinfoResult *results) +{ + (void)rv; + (void)num_results; + (void)results; +} + +void AddrinfoInjectSetUp(const MunitParameter params[]) +{ + (void)params; +} + +void AddrinfoInjectTearDown(void) +{ +} + +#else /* ifndef WITH_STATIC_DEPS */ + bool addrinfo_mock_enabled = false; enum addrinfo_mock_state { MockResultSet, MockResultReturned, SystemResult }; @@ -171,3 +193,5 @@ void freeaddrinfo(struct addrinfo *res) } free(response); } + +#endif /* ifdef WITH_STATIC_DEPS ... else */ diff --git a/test/raft/lib/addrinfo.h b/test/raft/lib/addrinfo.h index 7921412a9..beff503a7 100644 --- a/test/raft/lib/addrinfo.h +++ b/test/raft/lib/addrinfo.h @@ -1,14 +1,16 @@ -/* Support for getaddrinfo injection for test purpose +/* Support for getaddrinfo mocking for test purposes. * - * Provide a local bound version to capture teh getaddrinfo/freeaddrinfo - * incovation The helper may operate in three different modes: a) Transparent - * forward calls to system getaddrinfo/freeaddrinfo function, if the - * SET_UP_ADDRINFO/TEAR_DOWN_ADDRINFO is not added to the test test case setup - * teardown. b) Check, if all results requested by getaddrinfo are freed using - * freeaddrinfo. Activated by adding the SET_UP_ADDRINFO/SET_UP_ADDRINFO macros - * to the test fixture. c) Inject artifical responses into the the getaddrinfo - * requests for test purpose additionally to b) by using - * AddrinfoInjectSetResponse before triggering the getaddrinfo calls. + * The addrinfo.c test fixture includes definitions of getaddrinfo and + * freeaddinfo that override the libc definitions, adding usage checks and the + * ability to inject responses. These additional features are activated by + * adding SET_UP_ADDRINFO/TEAR_DOWN_ADDRINFO to the fixture constructor and + * destructor. + * + * The overriding definitions of getaddrinfo and freeaddrinfo affect all code + * that's linked with addrinfo.c, and we rely on being able to retrieve the + * original libc definitions using dlsym. When libc is statically linked, this + * is not possible, so we just arrange for the overriding definitions not to be + * compiled and skip any tests that rely on getaddrinfo result injection. */ #ifndef TEST_ADDRINFO_H @@ -16,14 +18,10 @@ #include "munit.h" -typedef struct AddrinfoResult -{ - const char *ip; - const int port; -} AddrinfoResult_t; - #ifdef DQLITE_STATIC_LIBC +/* Trickery to cause tests that use getaddrinfo result injection to be skipped + * when building with WITH_STATIC_DEPS. */ #define ADDRINFO_TEST(S, C, SETUP, TEAR_DOWN, OPTIONS, PARAMS) \ TEST(S, C, SETUP, TEAR_DOWN, OPTIONS, PARAMS) \ { \ @@ -32,23 +30,22 @@ typedef struct AddrinfoResult static MUNIT_UNUSED MunitResult test_unused_##S##_##C( \ MUNIT_UNUSED const MunitParameter params[], MUNIT_UNUSED void *data) -#define SET_UP_ADDRINFO -#define TEAR_DOWN_ADDRINFO -#define AddrinfoInjectSetResponse(a, b, c) \ - do { \ - (void)a; \ - (void)b; \ - (void)c; \ - } while (0) - #else /* ifndef DQLITE_STATIC_LIBC */ #define ADDRINFO_TEST(S, C, SETUP, TEAR_DOWN, OPTIONS, PARAMS) \ TEST(S, C, SETUP, TEAR_DOWN, OPTIONS, PARAMS) +#endif /* ifdef DQLITE_STATIC_LIBC ... else */ + #define SET_UP_ADDRINFO AddrinfoInjectSetUp(params) #define TEAR_DOWN_ADDRINFO AddrinfoInjectTearDown() +typedef struct AddrinfoResult +{ + const char *ip; + const int port; +} AddrinfoResult_t; + void AddrinfoInjectSetResponse(int rv, int num_results, const struct AddrinfoResult *results); @@ -56,6 +53,4 @@ void AddrinfoInjectSetResponse(int rv, void AddrinfoInjectSetUp(const MunitParameter params[]); void AddrinfoInjectTearDown(void); -#endif /* ifdef DQLITE_STATIC_LIBC ... else */ - #endif // #ifndef TEST_ADDRINFO_H From 1fb3a1fc59301809e8615544853a425a0598394c Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Sun, 1 Sep 2024 16:46:15 -0400 Subject: [PATCH 25/35] Fix Signed-off-by: Cole Miller --- .github/workflows/static.yml | 2 +- test/raft/lib/addrinfo.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/static.yml b/.github/workflows/static.yml index 10a0a32e7..1a0c9011c 100644 --- a/.github/workflows/static.yml +++ b/.github/workflows/static.yml @@ -19,4 +19,4 @@ jobs: env: LIBDQLITE_TRACE: 1 run: | - contrib/build-static.sh + contrib/build-static.sh || (cat ./test-suite.log && false) diff --git a/test/raft/lib/addrinfo.c b/test/raft/lib/addrinfo.c index 0cfad2a93..62fac96ce 100644 --- a/test/raft/lib/addrinfo.c +++ b/test/raft/lib/addrinfo.c @@ -7,7 +7,7 @@ #include #include -#ifdef WITH_STATIC_DEPS +#ifdef DQLITE_STATIC_LIBC void AddrinfoInjectSetResponse(int rv, int num_results, @@ -27,7 +27,7 @@ void AddrinfoInjectTearDown(void) { } -#else /* ifndef WITH_STATIC_DEPS */ +#else /* ifndef DQLITE_STATIC_LIBC */ bool addrinfo_mock_enabled = false; @@ -194,4 +194,4 @@ void freeaddrinfo(struct addrinfo *res) free(response); } -#endif /* ifdef WITH_STATIC_DEPS ... else */ +#endif /* ifdef DQLITE_STATIC_LIBC ... else */ From 6fdca1ed376165611c580f8eac419097125cf9f6 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Fri, 2 Aug 2024 12:01:12 -0500 Subject: [PATCH 26/35] sm: Initialize rc field Signed-off-by: Cole Miller --- src/lib/sm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/sm.c b/src/lib/sm.c index 5c1e696fd..426083219 100644 --- a/src/lib/sm.c +++ b/src/lib/sm.c @@ -49,6 +49,7 @@ void sm_init(struct sm *m, m->is_locked = is_locked; m->id = ++id; m->pid = getpid(); + m->rc = 0; snprintf(m->name, SM_MAX_NAME_LENGTH, "%s", name); sm_obs(m); From 6a8de74936df58811bae3ce59547491d8c8da2c6 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Fri, 2 Aug 2024 22:29:56 -0500 Subject: [PATCH 27/35] sm: Observe failures Signed-off-by: Cole Miller --- src/lib/sm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/sm.c b/src/lib/sm.c index 426083219..31840e7b5 100644 --- a/src/lib/sm.c +++ b/src/lib/sm.c @@ -83,6 +83,7 @@ void sm_fail(struct sm *m, int fail_state, int rc) m->rc = rc; m->state = fail_state; + sm_obs(m); POST(m->invariant != NULL && m->invariant(m, prev)); } From 73a600ef92789fcbd333727fcfd62bb7120fd28d Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Fri, 2 Aug 2024 22:53:58 -0500 Subject: [PATCH 28/35] sm: Remove extraneous newlines Signed-off-by: Cole Miller --- src/lib/sm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/sm.c b/src/lib/sm.c index 31840e7b5..e67d5a2ff 100644 --- a/src/lib/sm.c +++ b/src/lib/sm.c @@ -22,13 +22,13 @@ int sm_state(const struct sm *m) static inline void sm_obs(const struct sm *m) { - tracef("%s pid: %d sm_id: %" PRIu64 " %s |\n", + tracef("%s pid: %d sm_id: %" PRIu64 " %s |", m->name, m->pid, m->id, m->conf[sm_state(m)].name); } void sm_relate(const struct sm *from, const struct sm *to) { - tracef("%s-to-%s opid: %d dpid: %d id: %" PRIu64 " id: %" PRIu64 " |\n", + tracef("%s-to-%s opid: %d dpid: %d id: %" PRIu64 " id: %" PRIu64 " |", from->name, to->name, from->pid, to->pid, from->id, to->id); } @@ -89,7 +89,7 @@ void sm_fail(struct sm *m, int fail_state, int rc) static __attribute__((noinline)) bool check_failed(const char *f, int n, const char *s) { - tracef("%s:%d check failed: %s\n", f, n, s); + tracef("%s:%d check failed: %s", f, n, s); return false; } From 8e2d685e1077a188e527b1dcb6ba6c01fd76943b Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Fri, 2 Aug 2024 22:58:18 -0500 Subject: [PATCH 29/35] sm: Support attributes Signed-off-by: Cole Miller --- src/lib/sm.c | 11 +++++++++++ src/lib/sm.h | 5 +++++ 2 files changed, 16 insertions(+) diff --git a/src/lib/sm.c b/src/lib/sm.c index e67d5a2ff..9453c1fca 100644 --- a/src/lib/sm.c +++ b/src/lib/sm.c @@ -32,6 +32,17 @@ void sm_relate(const struct sm *from, const struct sm *to) from->name, to->name, from->pid, to->pid, from->id, to->id); } +void sm_attr(const struct sm *m, const char *k, const char *fmt, ...) +{ + char v[SM_MAX_ATTR_LENGTH]; + va_list ap; + va_start(ap, fmt); + vsnprintf(v, sizeof(v), fmt, ap); + va_end(ap); + tracef("%s-attr pid: %d sm_id: %" PRIu64 " %s %s |", + m->name, m->pid, m->id, k, v); +} + void sm_init(struct sm *m, bool (*invariant)(const struct sm *, int), bool (*is_locked)(const struct sm *), diff --git a/src/lib/sm.h b/src/lib/sm.h index e4c9b2a39..d7f72c23f 100644 --- a/src/lib/sm.h +++ b/src/lib/sm.h @@ -10,6 +10,7 @@ #define CHECK(cond) sm_check((cond), __FILE__, __LINE__, #cond) #define SM_MAX_NAME_LENGTH 50 +#define SM_MAX_ATTR_LENGTH 100 enum { SM_PREV_NONE = -1, @@ -54,5 +55,9 @@ int sm_state(const struct sm *m); bool sm_check(bool b, const char *f, int n, const char *s); /* Relates one state machine to another for observability. */ void sm_relate(const struct sm *from, const struct sm *to); +/** + * Records an attribute of a state machine for observability. + */ +void sm_attr(const struct sm *m, const char *k, const char *fmt, ...); #endif /* __LIB_SM__ */ From c65caf8dac4bbd6495e963f1667ef336e5326537 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Sun, 4 Aug 2024 23:18:50 -0400 Subject: [PATCH 30/35] Ignore format-nonliteral warning This fires on all our invocations of vsnprintf, but only with clang (gcc makes an exception for variadic functions presumably for just this reason). I think the value of this lint for us is not worth its price in \#pragma verbosity. Signed-off-by: Cole Miller --- configure.ac | 1 + src/logger.c | 3 --- test/lib/logger.c | 3 --- 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/configure.ac b/configure.ac index 6bc4afebc..c8cae742f 100644 --- a/configure.ac +++ b/configure.ac @@ -118,6 +118,7 @@ CC_CHECK_FLAGS_APPEND([AM_CFLAGS],[CFLAGS],[ \ -Wdate-time \ -Wnested-externs \ -Wconversion \ + -Wno-format-nonliteral \ -Werror \ ]) # To enable: diff --git a/src/logger.c b/src/logger.c index 92bdf919e..78614249d 100644 --- a/src/logger.c +++ b/src/logger.c @@ -36,10 +36,7 @@ void loggerDefaultEmit(void *data, int level, const char *fmt, va_list args) /* Then render the message, possibly truncating it. */ n = EMIT_BUF_LEN - strlen(buf) - 1; -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wformat-nonliteral" vsnprintf(cursor, n, fmt, args); -#pragma GCC diagnostic pop fprintf(stderr, "%s\n", buf); } diff --git a/test/lib/logger.c b/test/lib/logger.c index 2970254fa..20d2588c4 100644 --- a/test/lib/logger.c +++ b/test/lib/logger.c @@ -34,10 +34,7 @@ void test_logger_emit(void *data, int level, const char *format, va_list args) sprintf(buf + strlen(buf), "%2d -> [%s] ", t->id, level_name); -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wformat-nonliteral" vsnprintf(buf + strlen(buf), 1024 - strlen(buf), format, args); -#pragma GCC diagnostic pop munit_log(MUNIT_LOG_INFO, buf); return; From b74145c046d60404c671c7be58c1ef8e1e75851a Mon Sep 17 00:00:00 2001 From: Anatoliy Bilenko Date: Mon, 2 Sep 2024 08:20:17 +0000 Subject: [PATCH 31/35] Fixed query test. Added more iterations to uv_run() to deliver pool's callbacks --- test/unit/test_conn.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/test_conn.c b/test/unit/test_conn.c index f801b839d..cc8859a61 100644 --- a/test/unit/test_conn.c +++ b/test/unit/test_conn.c @@ -385,7 +385,7 @@ TEST_SETUP(query) PREPARE_CONN("CREATE TABLE test (n INT)", &stmt_id); EXEC_CONN(stmt_id, &f->last_insert_id, &f->rows_affected, 7); PREPARE_CONN("INSERT INTO test(n) VALUES (123)", &f->insert_stmt_id); - EXEC_CONN(f->insert_stmt_id, &f->last_insert_id, &f->rows_affected, 4); + EXEC_CONN(f->insert_stmt_id, &f->last_insert_id, &f->rows_affected, 9); return f; } From 128985cb6c10ff4d41d6f1f82cf616e7dd23648e Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Tue, 3 Sep 2024 11:25:49 -0400 Subject: [PATCH 32/35] Add README notes about static linking Signed-off-by: Cole Miller --- README.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/README.md b/README.md index b07760902..d9e587f72 100644 --- a/README.md +++ b/README.md @@ -119,6 +119,28 @@ The `--enable-build-raft` option causes dqlite to use its bundled Raft implementation instead of linking to an external libraft; the latter is a legacy configuration that should not be used for new development. +Building for static linking +--------------------------- + +If you're building dqlite for eventual use in a statically-linked +binary, there are some additional considerations. You should pass +`--with-static-deps` to the configure script; this disables code that +relies on dependencies being dynamically linked. (Currently it only +affects the test suite, but you should use it even when building +`libdqlite.a` only for future compatibility.) + +When linking libdqlite with musl libc, it's recommended to increase +the default stack size, which is otherwise too low for dqlite's +needs: + +``` +LDFLAGS="-Wl,-z,stack-size=1048576" +``` + +The `contrib/build-static.sh` script demonstrates building and +testing dqlite with all dependencies (including libc) statically +linked. + Usage Notes ----------- From 72ff0be6857c0f714b2d588bd108afb2cab3de43 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Thu, 5 Sep 2024 07:57:33 +0000 Subject: [PATCH 33/35] Propagate raft_recover errors and improve tracing At the moment, the raft_recover error messages are not propagated, so the caller gets a cryptic error code (1), without further details. This change ensures that the error messages are propagated and adds a few extra trace messages. Furthermore, tracing is disabled unless we start the node (not the case when we're actually trying to recover it). For now, we're adding a "dqliteTracingMaybeEnable" call in "dqlite_node_recover_ext". --- src/raft/raft.c | 1 + src/raft/uv.c | 11 +++++++++++ src/server.c | 7 +++++++ 3 files changed, 19 insertions(+) diff --git a/src/raft/raft.c b/src/raft/raft.c index ce3996b90..b0dd7d39f 100644 --- a/src/raft/raft.c +++ b/src/raft/raft.c @@ -227,6 +227,7 @@ int raft_recover(struct raft *r, const struct raft_configuration *conf) rv = r->io->recover(r->io, conf); if (rv != 0) { + ErrMsgTransfer(r->io->errmsg, r->errmsg, "io"); return rv; } diff --git a/src/raft/uv.c b/src/raft/uv.c index f1450e742..b093715e1 100644 --- a/src/raft/uv.c +++ b/src/raft/uv.c @@ -324,6 +324,7 @@ static int uvFilterSegments(struct uv *uv, ErrMsgPrintf(uv->io->errmsg, "closed segment %s is past last snapshot %s", segment->filename, snapshot_filename); + tracef("corrupted raft state, error: %s", uv->io->errmsg); return RAFT_CORRUPT; } @@ -369,6 +370,7 @@ static int uvLoadSnapshotAndEntries(struct uv *uv, rv = UvList(uv, &snapshots, &n_snapshots, &segments, &n_segments, uv->io->errmsg); if (rv != 0) { + tracef("failed to list snapshots and segments, error: %d", rv); goto err; } @@ -377,12 +379,14 @@ static int uvLoadSnapshotAndEntries(struct uv *uv, char snapshot_filename[UV__FILENAME_LEN]; *snapshot = RaftHeapMalloc(sizeof **snapshot); if (*snapshot == NULL) { + tracef("malloc failed"); rv = RAFT_NOMEM; goto err; } rv = UvSnapshotLoad(uv, &snapshots[n_snapshots - 1], *snapshot, uv->io->errmsg); if (rv != 0) { + tracef("snapshot load failed: %d", rv); RaftHeapFree(*snapshot); *snapshot = NULL; goto err; @@ -401,6 +405,7 @@ static int uvLoadSnapshotAndEntries(struct uv *uv, rv = uvFilterSegments(uv, (*snapshot)->index, snapshot_filename, &segments, &n_segments); if (rv != 0) { + tracef("failed to filter segments: %d", rv); goto err; } if (segments != NULL) { @@ -420,6 +425,7 @@ static int uvLoadSnapshotAndEntries(struct uv *uv, rv = uvSegmentLoadAll(uv, *start_index, segments, n_segments, entries, n); if (rv != 0) { + tracef("failed to load all segments: %d", rv); goto err; } @@ -447,6 +453,9 @@ static int uvLoadSnapshotAndEntries(struct uv *uv, err: assert(rv != 0); + tracef("auto-recovery: %d, load depth: %d, error: %s", + uv->auto_recovery, depth, uv->io->errmsg); + if (*snapshot != NULL) { snapshotDestroy(*snapshot); *snapshot = NULL; @@ -583,6 +592,7 @@ static int uvRecover(struct raft_io *io, const struct raft_configuration *conf) rv = uvLoadSnapshotAndEntries(uv, &snapshot, &start_index, &entries, &n_entries, 0); if (rv != 0) { + tracef("failed to load raft snapshot and entries, error: %d", rv); return rv; } @@ -599,6 +609,7 @@ static int uvRecover(struct raft_io *io, const struct raft_configuration *conf) rv = uvSegmentCreateClosedWithConfiguration(uv, next_index, conf); if (rv != 0) { + tracef("failed to create segment, error: %d", rv); return rv; } diff --git a/src/server.c b/src/server.c index 7e44b1c49..3f4f33fa5 100644 --- a/src/server.c +++ b/src/server.c @@ -1022,6 +1022,7 @@ int dqlite_node_recover_ext(dqlite_node *n, struct dqlite_node_info_ext infos[], int n_info) { + dqliteTracingMaybeEnable(true); tracef("dqlite node recover ext"); struct raft_configuration configuration; int i; @@ -1031,6 +1032,7 @@ int dqlite_node_recover_ext(dqlite_node *n, for (i = 0; i < n_info; i++) { struct dqlite_node_info_ext *info = &infos[i]; if (!node_info_valid(info)) { + tracef("invalid node info"); rv = DQLITE_MISUSE; goto out; } @@ -1040,6 +1042,7 @@ int dqlite_node_recover_ext(dqlite_node *n, rv = raft_configuration_add(&configuration, info->id, address, raft_role); if (rv != 0) { + tracef("unable to add server to raft configuration, error: %d", rv); assert(rv == RAFT_NOMEM); rv = DQLITE_NOMEM; goto out; @@ -1049,11 +1052,15 @@ int dqlite_node_recover_ext(dqlite_node *n, int lock_fd; rv = acquire_dir(n->config.raft_dir, &lock_fd); if (rv != 0) { + tracef("couldn't acquire lock, error: %d", rv); goto out; } rv = raft_recover(&n->raft, &configuration); if (rv != 0) { + tracef("raft recovery failed, error: %d", rv); + snprintf(n->errmsg, DQLITE_ERRMSG_BUF_SIZE, "raft_recover(): %s", + raft_errmsg(&n->raft)); rv = DQLITE_ERROR; goto out; } From 4b084b84f899449ab42d8694120869c4d13c8b25 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Mon, 16 Sep 2024 16:18:39 -0400 Subject: [PATCH 34/35] Try to work around GPG signing issue Signed-off-by: Cole Miller --- .github/workflows/packages.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/packages.yml b/.github/workflows/packages.yml index e5a39e8e1..62b319002 100644 --- a/.github/workflows/packages.yml +++ b/.github/workflows/packages.yml @@ -28,6 +28,11 @@ jobs: sudo apt-get update -qq sudo apt-get install -qq debhelper devscripts gnupg + - name: Work around GPG issue + run: | + mkdir -p ~/.gnupg + echo "pinentry-mode loopback" >~/.gnupg/gpg.conf + - name: Setup GPG signing key env: PPA_SECRET_KEY: ${{ secrets.PPA_SECRET_KEY }} From d79661ff0134952eed627de2b1c1d0463e30f3d0 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Mon, 16 Sep 2024 17:27:04 -0400 Subject: [PATCH 35/35] PPA: remove mantic and add oracular Signed-off-by: Cole Miller --- .github/workflows/packages.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/packages.yml b/.github/workflows/packages.yml index 62b319002..4a57f6e30 100644 --- a/.github/workflows/packages.yml +++ b/.github/workflows/packages.yml @@ -12,8 +12,8 @@ jobs: target: - focal - jammy - - mantic - noble + - oracular runs-on: ubuntu-20.04 environment: name: ppa