From e395138ba4560725ca0e47e472f1a17577c8a048 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Thu, 10 Nov 2005 09:58:10 +0000 Subject: [PATCH] - fixed memleak (ike_sa_id clone behavior) --- Source/charon/ike_sa_manager.c | 20 +++------ Source/charon/tests/ike_sa_manager_test.c | 52 +++++++++++++---------- 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/Source/charon/ike_sa_manager.c b/Source/charon/ike_sa_manager.c index 378f139e3..5bb9035ec 100644 --- a/Source/charon/ike_sa_manager.c +++ b/Source/charon/ike_sa_manager.c @@ -42,7 +42,7 @@ struct ike_sa_entry_s { */ int waiting_threads; /** - * is this SA flagged for deleting ? + * condvar where threads can wait until it's free again */ pthread_cond_t condvar; /** @@ -84,7 +84,7 @@ static status_t ike_sa_entry_destroy(ike_sa_entry_t *this) * * This constructor additionaly creates a new and empty SA * - * @param ike_sa_id the associated ike_sa_id_t, cloned + * @param ike_sa_id the associated ike_sa_id_t, will be cloned * @return created entry, with ike_sa and ike_sa_id */ static ike_sa_entry_t *ike_sa_entry_create(ike_sa_id_t *ike_sa_id) @@ -98,7 +98,7 @@ static ike_sa_entry_t *ike_sa_entry_create(ike_sa_id_t *ike_sa_id) this->checked_out = FALSE; this->driveout_new_threads = FALSE; this->driveout_waiting_threads = FALSE; - this->ike_sa_id = ike_sa_id; + ike_sa_id->clone(ike_sa_id, &(this->ike_sa_id)); this->ike_sa = ike_sa_create(ike_sa_id); return this; } @@ -343,18 +343,15 @@ static status_t checkout(private_ike_sa_manager_t *this, ike_sa_id_t *ike_sa_id, * IKE_SA. This could be improved... */ spi_t responder_spi; - ike_sa_id_t *new_ike_sa_id; ike_sa_entry_t *new_ike_sa_entry; /* set SPIs, we are the responder */ - ike_sa_id->clone(ike_sa_id, &new_ike_sa_id); this->get_next_spi(this, &responder_spi); - new_ike_sa_id->set_responder_spi(new_ike_sa_id, responder_spi); /* we also set arguments spi, so its still valid */ ike_sa_id->set_responder_spi(ike_sa_id, responder_spi); /* create entry */ - new_ike_sa_entry = ike_sa_entry_create(new_ike_sa_id); + new_ike_sa_entry = ike_sa_entry_create(ike_sa_id); this->list->insert_last(this->list, new_ike_sa_entry); /* check ike_sa out */ @@ -369,12 +366,8 @@ static status_t checkout(private_ike_sa_manager_t *this, ike_sa_id_t *ike_sa_id, /* creation of an IKE_SA from local site, * we are the initiator! */ - spi_t initiator_spi, responder_spi; - ike_sa_id_t *new_ike_sa_id; + spi_t initiator_spi; ike_sa_entry_t *new_ike_sa_entry; - - /* set SPIs */ - memset(&responder_spi, 0, sizeof(spi_t)); this->get_next_spi(this, &initiator_spi); @@ -382,8 +375,7 @@ static status_t checkout(private_ike_sa_manager_t *this, ike_sa_id_t *ike_sa_id, ike_sa_id->set_initiator_spi(ike_sa_id, initiator_spi); /* create entry */ - new_ike_sa_id = ike_sa_id_create(initiator_spi, responder_spi, INITIATOR); - new_ike_sa_entry = ike_sa_entry_create(new_ike_sa_id); + new_ike_sa_entry = ike_sa_entry_create(ike_sa_id); this->list->insert_last(this->list, new_ike_sa_entry); /* check ike_sa out */ diff --git a/Source/charon/tests/ike_sa_manager_test.c b/Source/charon/tests/ike_sa_manager_test.c index 198137850..a2c4873c3 100644 --- a/Source/charon/tests/ike_sa_manager_test.c +++ b/Source/charon/tests/ike_sa_manager_test.c @@ -35,7 +35,7 @@ static struct ike_sa_manager_test_struct_s { ike_sa_manager_t *isam; } td; -static void successful_thread(ike_sa_id_t *ike_sa_id) +static void test1_thread(ike_sa_id_t *ike_sa_id) { ike_sa_t *ike_sa; status_t status; @@ -47,23 +47,38 @@ static void successful_thread(ike_sa_id_t *ike_sa_id) td.tester->assert_true(td.tester, (status == SUCCESS), "checkin of a requested ike_sa"); } -static void failed_thread(ike_sa_id_t *ike_sa_id) + +static void test2_thread(ike_sa_id_t *ike_sa_id) +{ + ike_sa_t *ike_sa; + status_t status; + + status = td.isam->checkout(td.isam, ike_sa_id, &ike_sa); + td.tester->assert_true(td.tester, (status == NOT_FOUND), "IKE_SA already deleted"); +} + +static void test3_thread(ike_sa_id_t *ike_sa_id) { ike_sa_t *ike_sa; status_t status; status = td.isam->checkout(td.isam, ike_sa_id, &ike_sa); td.tester->assert_true(td.tester, (status == NOT_FOUND), "IKE_SA already deleted"); + + ike_sa_id->destroy(ike_sa_id); } + + + void test_ike_sa_manager(tester_t *tester) { status_t status; spi_t initiator, responder; - ike_sa_id_t *ike_sa_id; + ike_sa_id_t *ike_sa_id, *sa_id; ike_sa_t *ike_sa; int thread_count = 200; - int sa_count = 50; + int sa_count = 100; int i; pthread_t threads[thread_count]; @@ -74,7 +89,6 @@ void test_ike_sa_manager(tester_t *tester) - /* First Test: * we play initiator for IKE_SA_INIT first * create an IKE_SA, @@ -91,10 +105,10 @@ void test_ike_sa_manager(tester_t *tester) * this is usually done be the response from the communication partner, * but we don't have one... */ - ike_sa_id->destroy(ike_sa_id); - ike_sa_id = ike_sa->get_id(ike_sa); responder.low = 123; - ike_sa_id->set_responder_spi(ike_sa_id, responder); + + sa_id = ike_sa->get_id(ike_sa); + sa_id->set_responder_spi(sa_id, responder); /* check in, so we should have a "completed" sa, specified by ike_sa_id */ status = td.isam->checkin(td.isam, ike_sa); tester->assert_true(tester, (status == SUCCESS), "checkin modified IKE_SA"); @@ -102,12 +116,10 @@ void test_ike_sa_manager(tester_t *tester) /* now we check it out and start some other threads */ status = td.isam->checkout(td.isam, ike_sa_id, &ike_sa); tester->assert_true(tester, (status == SUCCESS), "checkout existing IKE_SA 1"); - - - + for (i = 0; i < thread_count; i++) { - if (pthread_create(&threads[i], NULL, (void*(*)(void*))successful_thread, (void*)ike_sa_id)) + if (pthread_create(&threads[i], NULL, (void*(*)(void*))test1_thread, (void*)ike_sa_id)) { /* failed, decrease list */ thread_count--; @@ -133,10 +145,7 @@ void test_ike_sa_manager(tester_t *tester) pthread_join(threads[i], NULL); } - //ike_sa_id->destroy(ike_sa_id); - - - + ike_sa_id->destroy(ike_sa_id); /* Second Test: @@ -144,7 +153,6 @@ void test_ike_sa_manager(tester_t *tester) * so we are the responder. * */ - memset(&initiator, 0, sizeof(initiator)); memset(&responder, 0, sizeof(responder)); @@ -155,7 +163,7 @@ void test_ike_sa_manager(tester_t *tester) tester->assert_true(tester, (status == SUCCESS), "checkout unexisting IKE_SA 2"); for (i = 0; i < thread_count; i++) { - if (pthread_create(&threads[i], NULL, (void*(*)(void*))failed_thread, (void*)ike_sa_id)) + if (pthread_create(&threads[i], NULL, (void*(*)(void*))test2_thread, (void*)ike_sa_id)) { /* failed, decrease list */ thread_count--; @@ -174,13 +182,13 @@ void test_ike_sa_manager(tester_t *tester) pthread_join(threads[i], NULL); } - //ike_sa_id->destroy(ike_sa_id); + ike_sa_id->destroy(ike_sa_id); /* Third Test: * put in a lot of IKE_SAs, check it out, set a thread waiting * and destroy the manager... */ - + memset(&initiator, 0, sizeof(initiator)); memset(&responder, 0, sizeof(responder)); @@ -194,12 +202,11 @@ void test_ike_sa_manager(tester_t *tester) status = td.isam->checkout(td.isam, ike_sa_id, &ike_sa); tester->assert_true(tester, (status == SUCCESS), "checkout unexisting IKE_SA 3"); - if (pthread_create(&threads[i], NULL, (void*(*)(void*))failed_thread, (void*)ike_sa_id)) + if (pthread_create(&threads[i], NULL, (void*(*)(void*))test3_thread, (void*)ike_sa_id)) { /* failed, decrease list */ thread_count--; } - //ike_sa_id->destroy(ike_sa_id); } /* let them go acquiring */ @@ -213,5 +220,6 @@ void test_ike_sa_manager(tester_t *tester) pthread_join(threads[i], NULL); } + }