- fixed memleak (ike_sa_id clone behavior)

diff --git a/Source/charon/ike_sa_manager.c b/Source/charon/ike_sa_manager.c
index 378f139..5bb9035 100644 (file)
--- 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 1981378..a2c4873 100644 (file)
--- 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);
        }
        
+       
 }