commit ec6c155f827000e337796f1f1c54299fbc5cf72a Author: Nick Mathewson <nickm@torproject.org> Date: Wed Jul 10 15:07:32 2013 -0400 Add some basic unit tests for the circuit map data structure. These show off the new mocking code by mocking the circuitmux code so that we can test the circuit map code in isolation. --- changes/fancy_testing | 5 +- src/or/circuitlist.c | 5 +- src/or/circuitlist.h | 6 ++ src/or/circuitmux.c | 10 +-- src/or/circuitmux.h | 8 ++- src/test/include.am | 1 + src/test/test.c | 2 + src/test/test_circuitlist.c | 168 +++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 191 insertions(+), 14 deletions(-) diff --git a/changes/fancy_testing b/changes/fancy_testing index 89876ff..fa5b570 100644 --- a/changes/fancy_testing +++ b/changes/fancy_testing @@ -22,7 +22,6 @@ stub functions, so that the tests can check the function without invoking the other functions it calls. - - - + - Add more unit tests for the <circid,channel>->circuit map, and + the destroy-cell-tracking code to fix bug 7912. diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index 1912b91..70c8980 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -8,7 +8,7 @@ * \file circuitlist.c * \brief Manage the global circuit list. **/ - +#define CIRCUITLIST_PRIVATE #include "or.h" #include "channel.h" #include "circuitbuild.h" @@ -41,7 +41,6 @@ circuit_t *global_circuitlist=NULL; /** A list of all the circuits in CIRCUIT_STATE_CHAN_WAIT. */ static smartlist_t *circuits_pending_chans = NULL; -static void circuit_free(circuit_t *circ); static void circuit_free_cpath(crypt_path_t *cpath); static void circuit_free_cpath_node(crypt_path_t *victim); static void cpath_ref_decref(crypt_path_reference_t *cpath_ref); @@ -736,7 +735,7 @@ or_circuit_new(circid_t p_circ_id, channel_t *p_chan) /** Deallocate space associated with circ. */ -static void +STATIC void circuit_free(circuit_t *circ) { void *mem; diff --git a/src/or/circuitlist.h b/src/or/circuitlist.h index 03f678c..a5a5485 100644 --- a/src/or/circuitlist.h +++ b/src/or/circuitlist.h @@ -12,6 +12,8 @@ #ifndef TOR_CIRCUITLIST_H #define TOR_CIRCUITLIST_H +#include "testsupport.h" + circuit_t * circuit_get_global_list_(void); const char *circuit_state_to_string(int state); const char *circuit_purpose_to_controller_string(uint8_t purpose); @@ -68,5 +70,9 @@ void circuits_handle_oom(size_t current_allocation); void channel_note_destroy_pending(channel_t *chan, circid_t id); void channel_note_destroy_not_pending(channel_t *chan, circid_t id); +#ifdef CIRCUITLIST_PRIVATE +STATIC void circuit_free(circuit_t *circ); +#endif + #endif diff --git a/src/or/circuitmux.c b/src/or/circuitmux.c index c84e0ce..0814163 100644 --- a/src/or/circuitmux.c +++ b/src/or/circuitmux.c @@ -922,9 +922,9 @@ circuitmux_num_circuits(circuitmux_t *cmux) * Attach a circuit to a circuitmux, for the specified direction. */ -void -circuitmux_attach_circuit(circuitmux_t *cmux, circuit_t *circ, - cell_direction_t direction) +MOCK_IMPL(void, +circuitmux_attach_circuit,(circuitmux_t *cmux, circuit_t *circ, + cell_direction_t direction)) { channel_t *chan = NULL; uint64_t channel_id; @@ -1071,8 +1071,8 @@ circuitmux_attach_circuit(circuitmux_t *cmux, circuit_t *circ, * no-op if not attached. */ -void -circuitmux_detach_circuit(circuitmux_t *cmux, circuit_t *circ) +MOCK_IMPL(void, +circuitmux_detach_circuit,(circuitmux_t *cmux, circuit_t *circ)) { chanid_circid_muxinfo_t search, *hashent = NULL; /* diff --git a/src/or/circuitmux.h b/src/or/circuitmux.h index a6bc415..ffa28be 100644 --- a/src/or/circuitmux.h +++ b/src/or/circuitmux.h @@ -10,6 +10,7 @@ #define TOR_CIRCUITMUX_H #include "or.h" +#include "testsupport.h" typedef struct circuitmux_policy_s circuitmux_policy_t; typedef struct circuitmux_policy_data_s circuitmux_policy_data_t; @@ -127,9 +128,10 @@ void circuitmux_notify_xmit_cells(circuitmux_t *cmux, circuit_t *circ, void circuitmux_notify_xmit_destroy(circuitmux_t *cmux); /* Circuit interface */ -void circuitmux_attach_circuit(circuitmux_t *cmux, circuit_t *circ, - cell_direction_t direction); -void circuitmux_detach_circuit(circuitmux_t *cmux, circuit_t *circ); +MOCK_DECL(void, circuitmux_attach_circuit, (circuitmux_t *cmux, circuit_t *circ, + cell_direction_t direction)); +MOCK_DECL(void, circuitmux_detach_circuit, + (circuitmux_t *cmux, circuit_t *circ)); void circuitmux_clear_num_cells(circuitmux_t *cmux, circuit_t *circ); void circuitmux_set_num_cells(circuitmux_t *cmux, circuit_t *circ, unsigned int n_cells); diff --git a/src/test/include.am b/src/test/include.am index 989cf4e..279369e 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -19,6 +19,7 @@ src_test_test_SOURCES = \ src/test/test.c \ src/test/test_addr.c \ src/test/test_cell_formats.c \ + src/test/test_circuitlist.c \ src/test/test_containers.c \ src/test/test_crypto.c \ src/test/test_data.c \ diff --git a/src/test/test.c b/src/test/test.c index 7721d2c..d7d4c6c 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -2132,6 +2132,7 @@ extern struct testcase_t config_tests[]; extern struct testcase_t introduce_tests[]; extern struct testcase_t replaycache_tests[]; extern struct testcase_t cell_format_tests[]; +extern struct testcase_t circuitlist_tests[]; static struct testgroup_t testgroups[] = { { "", test_array }, @@ -2147,6 +2148,7 @@ static struct testgroup_t testgroups[] = { { "config/", config_tests }, { "replaycache/", replaycache_tests }, { "introduce/", introduce_tests }, + { "circuitlist/", circuitlist_tests }, END_OF_GROUPS }; diff --git a/src/test/test_circuitlist.c b/src/test/test_circuitlist.c new file mode 100644 index 0000000..718f0cd --- /dev/null +++ b/src/test/test_circuitlist.c @@ -0,0 +1,168 @@ +/* Copyright (c) 2013, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +#define TOR_CHANNEL_INTERNAL_ +#define CIRCUITLIST_PRIVATE +#include "or.h" +#include "channel.h" +#include "circuitlist.h" +#include "test.h" + +static channel_t * +new_fake_channel(void) +{ + channel_t *chan = tor_malloc_zero(sizeof(channel_t)); + channel_init(chan); + return chan; +} + +static struct { + int ncalls; + void *cmux; + void *circ; + cell_direction_t dir; +} cam; + +static void +circuitmux_attach_mock(circuitmux_t *cmux, circuit_t *circ, + cell_direction_t dir) +{ + ++cam.ncalls; + cam.cmux = cmux; + cam.circ = circ; + cam.dir = dir; +} + +static struct { + int ncalls; + void *cmux; + void *circ; +} cdm; + +static void +circuitmux_detach_mock(circuitmux_t *cmux, circuit_t *circ) +{ + ++cdm.ncalls; + cdm.cmux = cmux; + cdm.circ = circ; +} + +#define GOT_CMUX_ATTACH(mux_, circ_, dir_) do { \ + tt_int_op(cam.ncalls, ==, 1); \ + tt_ptr_op(cam.cmux, ==, (mux_)); \ + tt_ptr_op(cam.circ, ==, (circ_)); \ + tt_ptr_op(cam.dir, ==, (dir_)); \ + memset(&cam, 0, sizeof(cam)); \ + } while (0) + +#define GOT_CMUX_DETACH(mux_, circ_) do { \ + tt_int_op(cdm.ncalls, ==, 1); \ + tt_ptr_op(cdm.cmux, ==, (mux_)); \ + tt_ptr_op(cdm.circ, ==, (circ_)); \ + memset(&cdm, 0, sizeof(cdm)); \ + } while (0) + + +static void +test_clist_maps(void *arg) +{ + channel_t *ch1 = new_fake_channel(); + channel_t *ch2 = new_fake_channel(); + channel_t *ch3 = new_fake_channel(); + or_circuit_t *or_c1=NULL, *or_c2=NULL; + + MOCK(circuitmux_attach_circuit, circuitmux_attach_mock); + MOCK(circuitmux_detach_circuit, circuitmux_detach_mock); + memset(&cam, 0, sizeof(cam)); + memset(&cdm, 0, sizeof(cdm)); + + ch1->cmux = (void*)0x1001; + ch2->cmux = (void*)0x1002; + ch3->cmux = (void*)0x1003; + + or_c1 = or_circuit_new(100, ch2); + tt_assert(or_c1); + GOT_CMUX_ATTACH(ch2->cmux, or_c1, CELL_DIRECTION_IN); + tt_int_op(or_c1->p_circ_id, ==, 100); + tt_ptr_op(or_c1->p_chan, ==, ch2); + + or_c2 = or_circuit_new(100, ch1); + tt_assert(or_c2); + GOT_CMUX_ATTACH(ch1->cmux, or_c2, CELL_DIRECTION_IN); + tt_int_op(or_c2->p_circ_id, ==, 100); + tt_ptr_op(or_c2->p_chan, ==, ch1); + + circuit_set_n_circid_chan(TO_CIRCUIT(or_c1), 200, ch1); + GOT_CMUX_ATTACH(ch1->cmux, or_c1, CELL_DIRECTION_OUT); + + circuit_set_n_circid_chan(TO_CIRCUIT(or_c2), 200, ch2); + GOT_CMUX_ATTACH(ch2->cmux, or_c2, CELL_DIRECTION_OUT); + + tt_ptr_op(circuit_get_by_circid_channel(200, ch1), ==, TO_CIRCUIT(or_c1)); + tt_ptr_op(circuit_get_by_circid_channel(200, ch2), ==, TO_CIRCUIT(or_c2)); + tt_ptr_op(circuit_get_by_circid_channel(100, ch2), ==, TO_CIRCUIT(or_c1)); + /* Try the same thing again, to test the "fast" path. */ + tt_ptr_op(circuit_get_by_circid_channel(100, ch2), ==, TO_CIRCUIT(or_c1)); + tt_assert(circuit_id_in_use_on_channel(100, ch2)); + tt_assert(! circuit_id_in_use_on_channel(101, ch2)); + + /* Try changing the circuitid and channel of that circuit. */ + circuit_set_p_circid_chan(or_c1, 500, ch3); + GOT_CMUX_DETACH(ch2->cmux, TO_CIRCUIT(or_c1)); + GOT_CMUX_ATTACH(ch3->cmux, TO_CIRCUIT(or_c1), CELL_DIRECTION_IN); + tt_ptr_op(circuit_get_by_circid_channel(100, ch2), ==, NULL); + tt_assert(! circuit_id_in_use_on_channel(100, ch2)); + tt_ptr_op(circuit_get_by_circid_channel(500, ch3), ==, TO_CIRCUIT(or_c1)); + + /* Now let's see about destroy handling. */ + tt_assert(! circuit_id_in_use_on_channel(205, ch2)); + tt_assert(circuit_id_in_use_on_channel(200, ch2)); + channel_note_destroy_pending(ch2, 200); + channel_note_destroy_pending(ch2, 205); + channel_note_destroy_pending(ch1, 100); + tt_assert(circuit_id_in_use_on_channel(205, ch2)) + tt_assert(circuit_id_in_use_on_channel(200, ch2)); + tt_assert(circuit_id_in_use_on_channel(100, ch1)); + + tt_assert(TO_CIRCUIT(or_c2)->n_delete_pending != 0); + tt_ptr_op(circuit_get_by_circid_channel(200, ch2), ==, TO_CIRCUIT(or_c2)); + tt_ptr_op(circuit_get_by_circid_channel(100, ch1), ==, TO_CIRCUIT(or_c2)); + + /* Okay, now free ch2 and make sure that the circuit ID is STILL not + * usable, because we haven't declared the destroy to be nonpending */ + tt_int_op(cdm.ncalls, ==, 0); + circuit_free(TO_CIRCUIT(or_c2)); + or_c2 = NULL; /* prevent free */ + tt_int_op(cdm.ncalls, ==, 2); + memset(&cdm, 0, sizeof(cdm)); + tt_assert(circuit_id_in_use_on_channel(200, ch2)); + tt_assert(circuit_id_in_use_on_channel(100, ch1)); + tt_ptr_op(circuit_get_by_circid_channel(200, ch2), ==, NULL); + tt_ptr_op(circuit_get_by_circid_channel(100, ch1), ==, NULL); + + /* Now say that the destroy is nonpending */ + channel_note_destroy_not_pending(ch2, 200); + tt_ptr_op(circuit_get_by_circid_channel(200, ch2), ==, NULL); + channel_note_destroy_not_pending(ch1, 100); + tt_ptr_op(circuit_get_by_circid_channel(100, ch1), ==, NULL); + tt_assert(! circuit_id_in_use_on_channel(200, ch2)); + tt_assert(! circuit_id_in_use_on_channel(100, ch1)); + + +done: + tor_free(ch1); + tor_free(ch2); + tor_free(ch3); + if (or_c1) + circuit_free(TO_CIRCUIT(or_c1)); + if (or_c2) + circuit_free(TO_CIRCUIT(or_c2)); + UNMOCK(circuitmux_attach_circuit); + UNMOCK(circuitmux_detach_circuit); +} + + +struct testcase_t circuitlist_tests[] = { + { "maps", test_clist_maps, TT_FORK, NULL, NULL }, + END_OF_TESTCASES +};