implemented proper refcounting using atomic operations
authorMartin Willi <martin@strongswan.org>
Fri, 28 Jul 2006 09:45:18 +0000 (09:45 -0000)
committerMartin Willi <martin@strongswan.org>
Fri, 28 Jul 2006 09:45:18 +0000 (09:45 -0000)
src/charon/config/connections/connection.c
src/charon/config/connections/connection.h
src/charon/config/policies/policy.c
src/charon/config/policies/policy.h
src/charon/daemon.c
src/charon/doc/Todo-list.txt
src/libstrongswan/types.c
src/libstrongswan/types.h

index 3ecc08a..281d43d 100644 (file)
@@ -63,7 +63,7 @@ struct private_connection_t {
        /**
         * Number of references hold by others to this connection
         */
-       u_int refcount;
+       refcount_t refcount;
 
        /**
         * Name of the connection
@@ -326,7 +326,7 @@ static u_int32_t get_hard_lifetime(private_connection_t *this)
  */
 static void get_ref(private_connection_t *this)
 {
-       this->refcount++;
+       ref_get(&this->refcount);
 }
 
 /**
@@ -334,7 +334,7 @@ static void get_ref(private_connection_t *this)
  */
 static void destroy(private_connection_t *this)
 {
-       if (--this->refcount == 0)
+       if (ref_put(&this->refcount))
        {
                proposal_t *proposal;
                
index 068dd71..61ca9cf 100644 (file)
@@ -259,6 +259,9 @@ struct connection_t {
         *
         * Get a new reference to this connection by increasing
         * it's internal reference counter.
+        * Do not call get_ref or any other function until you
+        * already have a reference. Otherwise the object may get
+        * destroyed while calling get_ref(),
         *
         * @param this          calling object
         */
index 14fa93b..49983ad 100644 (file)
@@ -46,7 +46,7 @@ struct private_policy_t {
        /**
         * Number of references hold by others to this policy
         */
-       u_int refcount;
+       refcount_t refcount;
        
        /**
         * Name of the policy, used to query it
@@ -377,7 +377,7 @@ static u_int32_t get_hard_lifetime(private_policy_t *this)
  */
 static void get_ref(private_policy_t *this)
 {
-       this->refcount++;
+       ref_get(&this->refcount);
 }
 
 /**
@@ -385,7 +385,7 @@ static void get_ref(private_policy_t *this)
  */
 static void destroy(private_policy_t *this)
 {
-       if (--this->refcount == 0)
+       if (ref_put(&this->refcount))
        {
                proposal_t *proposal;
                traffic_selector_t *traffic_selector;
index 1659a70..454f69c 100644 (file)
@@ -227,6 +227,9 @@ struct policy_t {
         *
         * Get a new reference to this policy by increasing
         * it's internal reference counter.
+        * Do not call get_ref or any other function until you
+        * already have a reference. Otherwise the object may get
+        * destroyed while calling get_ref(),
         * 
         * @param this                          calling object
         */
index 5e4a67a..a368ee4 100644 (file)
@@ -32,6 +32,7 @@
 #include <execinfo.h>
 #include <string.h>
 #include <getopt.h>
+#include <errno.h>
 
 #include "daemon.h" 
 
index 788a06c..296820e 100644 (file)
@@ -54,6 +54,8 @@
 + proper delete messages
 - notifys on connection setup failure
 + create child sa message/rekeying
+/ IKE_SA rekeying
+  - handle all simultaneous rekeying/delete/create cases
 
 - implement a mechanism against thread exhaustion
   when a blocked IKE_SA receives a lot of messages
index b60a604..e033727 100644 (file)
@@ -25,6 +25,7 @@
 #include <time.h>
 #include <stdio.h>
 #include <stdarg.h>
+#include <pthread.h>
 
 #include "types.h"
 
@@ -198,6 +199,43 @@ void *clalloc(void * pointer, size_t size)
        return (data);
 }
 
+/**
+ * We use a single mutex for all refcount variables. This
+ * is not optimal for performance, but the critical section
+ * is not that long...
+ * TODO: Consider to include a mutex in each refcount_t variable.
+ */
+static pthread_mutex_t ref_mutex = PTHREAD_MUTEX_INITIALIZER;
+
+/**
+ * Described in header.
+ * 
+ * TODO: May be implemented with atomic CPU instructions
+ * instead of a mutex.
+ */
+void ref_get(refcount_t *ref)
+{
+       pthread_mutex_lock(&ref_mutex);
+       (*ref)++;
+       pthread_mutex_unlock(&ref_mutex);
+}
+
+/**
+ * Described in header.
+ * 
+ * TODO: May be implemented with atomic CPU instructions
+ * instead of a mutex.
+ */
+bool ref_put(refcount_t *ref)
+{
+       bool more_refs;
+       
+       pthread_mutex_lock(&ref_mutex);
+       more_refs = --(*ref);
+       pthread_mutex_unlock(&ref_mutex);
+       return !more_refs;
+}
+
 /*
  * Names of the months used by timetoa()
  */
index 631e502..9874d5b 100644 (file)
@@ -200,6 +200,32 @@ void chunk_to_hex(char *buf, size_t buflen, chunk_t chunk);
  */
 void *clalloc(void *pointer, size_t size);
 
+/**
+ * Special type to count references
+ */
+typedef volatile u_int refcount_t;
+
+/**
+ * @brief Get a new reference.
+ *
+ * Increments the reference counter atomic.
+ *
+ * @param ref  pointer to ref counter
+ */
+void ref_get(refcount_t *ref);
+
+/**
+ * @brief Put back a unused reference.
+ *
+ * Decrements the reference counter atomic and 
+ * says if more references available.
+ *
+ * @param ref  pointer to ref counter
+ * @return             TRUE if no more references counted
+ */
+bool ref_put(refcount_t *ref);
+
+
 #define UNDEFINED_TIME 0
 #define TIMETOA_BUF    30