diff --git a/configure.ac b/configure.ac index dec472c..7f2fc2a 100644 --- a/configure.ac +++ b/configure.ac @@ -10,7 +10,7 @@ AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_AUX_DIR([build-aux]) AC_CONFIG_HEADERS([imaptest-config.h]) -AM_INIT_AUTOMAKE +AM_INIT_AUTOMAKE([serial-tests]) AM_SILENT_RULES([yes]) AM_MAINTAINER_MODE diff --git a/src/Makefile.am b/src/Makefile.am index f896e42..3d41394 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1,16 +1,17 @@ bin_PROGRAMS = imaptest noinst_PROGRAMS = test-test-parser +check_PROGRAMS = test-client-stalled +TESTS = $(check_PROGRAMS) AM_CPPFLAGS = $(LIBDOVECOT_INCLUDE) -DSTATIC_OPENSSL=1 imaptest_LDFLAGS = $(AM_LDFLAGS) -imaptest_SOURCES = \ +common_sources = \ checkpoint.c \ client.c \ client-state.c \ commands.c \ imap-client.c \ - imaptest.c \ imaptest-lmtp.c \ mailbox.c \ mailbox-source.c \ @@ -26,10 +27,20 @@ imaptest_SOURCES = \ test-parser.c \ user.c +imaptest_SOURCES = \ + $(common_sources) \ + imaptest.c + test_test_parser_SOURCES = test-test-parser.c test-parser.c settings.c test_test_parser_LDADD = $(imaptest_LDADD) test_test_parser_CFLAGS = $(imaptest_CFLAGS) +test_client_stalled_SOURCES = \ + $(common_sources) \ + test-client-stalled.c +test_client_stalled_LDADD = $(imaptest_LDADD) +test_client_stalled_CFLAGS = $(imaptest_CFLAGS) + noinst_HEADERS = \ checkpoint.h \ client.h \ diff --git a/src/client.c b/src/client.c index 3f44524..3f7d3a6 100644 --- a/src/client.c +++ b/src/client.c @@ -30,7 +30,7 @@ int clients_count = 0; unsigned int total_disconnects = 0; ARRAY_TYPE(client) clients; -ARRAY(unsigned int) stalled_clients; +ARRAY_TYPE(unsigned_int) stalled_clients; bool stalled = FALSE, disconnect_clients = FALSE, scripted_tests_running = FALSE; static unsigned int client_min_free_idx = 0; @@ -198,21 +198,23 @@ client_new_full(unsigned int i, struct user *user, struct user_client *uc) { struct imap_client *imap_c; struct pop3_client *pop3_c; - - if (client_min_free_idx == i) - client_min_free_idx++; + struct client *client; if (uc == NULL || uc->profile == NULL || strcmp(uc->profile->protocol, "imap") == 0) { if (imap_client_new(i, user, uc, &imap_c) < 0) return NULL; - return &imap_c->client; + client = &imap_c->client; } else if (strcmp(uc->profile->protocol, "pop3") == 0) { if (pop3_client_new(i, user, uc, &pop3_c) < 0) return NULL; - return &pop3_c->client; + client = &pop3_c->client; } else i_unreached(); + + if (client_min_free_idx == client->idx) + client_min_free_idx++; + return client; } struct client *client_new_user(struct user *user) @@ -249,8 +251,25 @@ int client_init(struct client *client, unsigned int idx, const struct ip_addr *ip; int fd; + if (idx < array_count(&clients) && + array_idx_elem(&clients, idx) != NULL) { + /* The requested index is already occupied. This can happen + when clients_unstalled() reuses a stalled client's index + that was subsequently reassigned to another client. + Find a free slot. */ + struct client *const *c; + unsigned int i, count; + + c = array_get(&clients, &count); + for (i = 0; i < count; i++) { + if (c[i] == NULL) + break; + } + idx = i; + } + i_assert(idx >= array_count(&clients) || - *(struct client **)array_idx(&clients, idx) == NULL); + array_idx_elem(&clients, idx) == NULL); if (stalled) { array_append(&stalled_clients, &idx, 1); return -1; @@ -429,7 +448,8 @@ unsigned int clients_get_random_idx(void) void clients_init(void) { - i_array_init(&stalled_clients, CLIENTS_COUNT); + i_array_init(&clients, conf.clients_count); + i_array_init(&stalled_clients, conf.clients_count); } void clients_deinit(void) diff --git a/src/client.h b/src/client.h index 72358b7..65e5f6c 100644 --- a/src/client.h +++ b/src/client.h @@ -51,10 +51,12 @@ struct client { bool idling:1; }; ARRAY_DEFINE_TYPE(client, struct client *); +ARRAY_DEFINE_TYPE(unsigned_int, unsigned int); extern int clients_count; extern unsigned int total_disconnects; extern ARRAY_TYPE(client) clients; +extern ARRAY_TYPE(unsigned_int) stalled_clients; extern bool stalled, disconnect_clients, scripted_tests_running; struct client *client_new_user(struct user *user); diff --git a/src/imaptest.c b/src/imaptest.c index bcc1a39..b930054 100644 --- a/src/imaptest.c +++ b/src/imaptest.c @@ -862,7 +862,6 @@ int main(int argc ATTR_UNUSED, char *argv[]) ssl_iostream_openssl_init(); #endif - i_array_init(&clients, CLIENTS_COUNT); if (testpath == NULL) imaptest_run(); else diff --git a/src/test-client-stalled.c b/src/test-client-stalled.c new file mode 100644 index 0000000..882af0a --- /dev/null +++ b/src/test-client-stalled.c @@ -0,0 +1,176 @@ +/* Copyright (c) ImapTest authors, see the included COPYING file */ + +#include "lib.h" +#include "ioloop.h" +#include "array.h" +#include "settings.h" +#include "client.h" +#include "imap-client.h" +#include "user.h" +#include "mailbox.h" +#include "mailbox-source.h" +#include "test-common.h" + +struct settings conf; +bool profile_running = FALSE; + +/* Mock net_connect_ip to return a real (but unconnected) socket. + This allows using real i_stream_create_fd() etc. without real network. + Since we link against libdovecot, we can provide our own version of this + function to override the one in the library for this test. */ +int net_connect_ip(const struct ip_addr *ip, in_port_t port, const struct ip_addr *my_ip) +{ + return socket(AF_INET, SOCK_STREAM, 0); +} + +/* These are needed by imap-client.c and other files but are defined in imaptest.c */ +bool imaptest_has_clients(void) { return FALSE; } +void sig_die(int signo ATTR_UNUSED, void *context ATTR_UNUSED) { exit(1); } +void error_quit(void) { exit(1); } + +static void test_stalled_client_reassignment(void) +{ + struct user *user; + struct imap_client *c1, *c2, *c3; + struct ioloop *ioloop; + struct mailbox_source *source; + + test_begin("stalled client index reassignment"); + + ioloop = io_loop_create(); + + conf.clients_count = 10; + conf.ips_count = 1; + conf.ips = i_new(struct ip_addr, 1); + conf.mailbox = "INBOX"; + + clients_init(); + mailboxes_init(); + + source = mailbox_source_new_random(1024); + users_init(NULL, source); + user = user_get("testuser", source); + + /* 1. Add a client at index 0 */ + if (imap_client_new(0, user, NULL, &c1) < 0) + i_fatal("imap_client_new 1 failed"); + + test_assert(array_count(&clients) == 1); + test_assert(array_idx_elem(&clients, 0) == &c1->client); + + /* 2. Set stalled = TRUE, then try to init another client at index 0. + This simulates clients_unstalled() trying to reuse index 0. */ + stalled = TRUE; + /* In the fixed code, client_init should have found that index 0 is occupied, + found index 1 is free, and appended index 1 to stalled_clients. */ + if (imap_client_new(0, user, NULL, &c2) != -1) + i_fatal("imap_client_new 2 should have stalled and returned -1"); + + const unsigned int *stalled_idxs; + unsigned int stalled_count; + stalled_idxs = array_get(&stalled_clients, &stalled_count); + test_assert(stalled_count == 1); + test_assert(stalled_idxs[0] == 1); + + /* 3. Now let's occupy index 1 as well */ + stalled = FALSE; + if (imap_client_new(1, user, NULL, &c3) < 0) + i_fatal("imap_client_new 3 failed"); + + test_assert(array_count(&clients) == 2); + test_assert(array_idx_elem(&clients, 1) == &c3->client); + + /* 4. Now try to stall index 0 again. It should find index 2. */ + stalled = TRUE; + if (imap_client_new(0, user, NULL, &c2) != -1) + i_fatal("imap_client_new 4 should have stalled and returned -1"); + + stalled_idxs = array_get(&stalled_clients, &stalled_count); + test_assert(stalled_count == 2); + test_assert(stalled_idxs[1] == 2); + + client_unref(&c1->client, FALSE); + client_unref(&c3->client, FALSE); + + users_deinit(); + mailbox_source_unref(&source); + mailboxes_deinit(); + clients_deinit(); + array_free(&clients); + i_free(conf.ips); + + io_loop_destroy(&ioloop); + + test_end(); +} + +static void test_nonstalled_client_reassignment(void) +{ + struct user *user; + struct imap_client *c1, *c2; + struct ioloop *ioloop; + struct mailbox_source *source; + + test_begin("non-stalled client index reassignment"); + + ioloop = io_loop_create(); + + conf.clients_count = 10; + conf.ips_count = 1; + conf.ips = i_new(struct ip_addr, 1); + conf.mailbox = "INBOX"; + + clients_init(); + mailboxes_init(); + + source = mailbox_source_new_random(1024); + users_init(NULL, source); + user = user_get("testuser", source); + + /* 1. Add a client at index 0 */ + if (imap_client_new(0, user, NULL, &c1) < 0) + i_fatal("imap_client_new 1 failed"); + + test_assert(array_count(&clients) == 1); + test_assert(c1->client.idx == 0); + + /* 2. With stalled = FALSE, request index 0 which is occupied. + client_init should find index 1 free and create the client there. */ + stalled = FALSE; + if (imap_client_new(0, user, NULL, &c2) < 0) + i_fatal("imap_client_new 2 failed"); + + test_assert(array_count(&clients) == 2); + test_assert(c2->client.idx == 1); + test_assert(array_idx_elem(&clients, 1) == &c2->client); + + /* 3. Verify stalled_clients array is empty - nothing was stalled */ + const unsigned int *stalled_idxs; + unsigned int stalled_count; + stalled_idxs = array_get(&stalled_clients, &stalled_count); + test_assert(stalled_count == 0); + + client_unref(&c1->client, FALSE); + client_unref(&c2->client, FALSE); + + users_deinit(); + mailbox_source_unref(&source); + mailboxes_deinit(); + clients_deinit(); + array_free(&clients); + i_free(conf.ips); + + io_loop_destroy(&ioloop); + + test_end(); +} + +int main(void) +{ + static void (*const test_functions[])(void) = { + test_stalled_client_reassignment, + test_nonstalled_client_reassignment, + NULL + }; + return test_run(test_functions); +}