Changeset 924:877592751fee in freeDiameter for libfdproto/sessions.c

Timestamp:

Feb 15, 2013, 1:52:57 AM (11 years ago)

Author:

Sebastien Decugis <sdecugis@freediameter.net>

Branch:

default

Children:

925:e5a09fab5ef3, 950:51c15f98a965

Phase:

public

Message:

Fix (tentative) for invalid handling of sessions fast creation/destruction as pointed by Yusuke Okura -- ​http://lists.freediameter.net/pipermail/help/2013-February/000584.html and ​http://lists.freediameter.net/pipermail/help/2013-February/000589.html -- Thank you very much

File:

• libfdproto/sessions.c (modified) (9 diffs)

libfdproto/sessions.c

@@ -360 +360 @@
 
 
-/* Create a new session object with the default timeout value, and link it */
+/* Create a new session object with the default timeout value, and link it. The refcount is increased by 1, whether the session existed or not */
 int fd_sess_new ( struct session ** session, DiamId_t diamid, size_t diamidlen, uint8_t * opt, size_t optlen )
 {
@@ -461 +461 @@
         
                 fd_list_insert_before(li, &sess->chain_h); /* hash table */
+                sess->msg_cnt++;
         } else {
                 free(sid);
+                
+                CHECK_POSIX( pthread_mutex_lock(&(*session)->stlock) ); 
+                (*session)->msg_cnt++;
+                CHECK_POSIX( pthread_mutex_unlock(&(*session)->stlock) ); 
+                
                 /* it was found: was it previously destroyed? */
                 if ((*session)->is_destroyed == 0) {
@@ -469 +475 @@
                 } else {
                         /* the session was marked destroyed, let's re-activate it. */
-                        TODO("Re-creating a deleted session. Should investigate if this can lead to an issue... (need more feedback)");
                         sess = *session;
+                        sess->is_destroyed = 0;
                         
                         /* update the expiry time */
@@ -501 +507 @@
 
 out:
-        ;       
+        ;
         pthread_cleanup_pop(0);
         CHECK_POSIX( pthread_mutex_unlock( H_LOCK(hash) ) );
@@ -512 +518 @@
 }
 
-/* Find or create a session */
-int fd_sess_fromsid ( uint8_t * sid, size_t len, struct session ** session, int * new)
+/* Find or create a session -- the msg refcount is increased */
+int fd_sess_fromsid_msg ( uint8_t * sid, size_t len, struct session ** session, int * new)
 {
         int ret;
@@ -802 +808 @@
 
 /* For the messages module */
-int fd_sess_fromsid_msg ( uint8_t * sid, size_t len, struct session ** session, int * new)
+int fd_sess_fromsid ( uint8_t * sid, size_t len, struct session ** session, int * new)
 {
         TRACE_ENTRY("%p %zd %p %p", sid, len, session, new);
@@ -808 +814 @@
         
         /* Get the session object */
-        CHECK_FCT( fd_sess_fromsid ( sid, len, session, new) );
-        
-        /* Increase count */
-        CHECK_FCT( fd_sess_ref_msg ( *session ) );
-        
+        CHECK_FCT( fd_sess_fromsid_msg ( sid, len, session, new) );
+        
+        /* Decrease the refcount */
+        CHECK_POSIX( pthread_mutex_lock(&(*session)->stlock) );
+        (*session)->msg_cnt--; /* was increased in fd_sess_new */
+        CHECK_POSIX( pthread_mutex_unlock(&(*session)->stlock) );
+                
         /* Done */
         return 0;
@@ -833 +841 @@
 {
         int reclaim;
+        uint32_t hash;
         
         TRACE_ENTRY("%p", session);
         CHECK_PARAMS( session && VALIDATE_SI(*session) );
         
+        /* Lock the hash line to avoid possibility that session is freed while we are reclaiming */
+        hash = (*session)->hash;
+        CHECK_POSIX( pthread_mutex_lock( H_LOCK(hash)) );
+        pthread_cleanup_push( fd_cleanup_mutex, H_LOCK(hash) ); 
+
         /* Update the msg refcount */
         CHECK_POSIX( pthread_mutex_lock(&(*session)->stlock) );
@@ -843 +857 @@
         CHECK_POSIX( pthread_mutex_unlock(&(*session)->stlock) );
         
+        /* Ok, now unlock the hash line */
+        pthread_cleanup_pop( 0 );
+        CHECK_POSIX( pthread_mutex_unlock( H_LOCK(hash) ) );
+        
+        /* and reclaim if no message references the session anymore */
         if (reclaim == 1) {
                 CHECK_FCT(fd_sess_reclaim ( session ));