Changeset 691:78b665400097 in freeDiameter for libfdproto/sessions.c

Timestamp:

Jan 20, 2011, 7:44:27 PM (13 years ago)

Author:

Sebastien Decugis <sdecugis@nict.go.jp>

Branch:

default

Phase:

public

Message:

Cleanup all pthread_cleanup_push / pop pairs so that pop is always called after push, or ASSERT(0) is some grave errors

File:

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

libfdproto/sessions.c

@@ -176 +176 @@
                 if (FD_IS_LIST_EMPTY(&exp_sentinel)) {
                         /* Just wait for a change or cancelation */
-                        CHECK_POSIX_DO( pthread_cond_wait( &exp_cond, &exp_lock ), break );
+                        CHECK_POSIX_DO( pthread_cond_wait( &exp_cond, &exp_lock ), break /* this might not pop the cleanup handler, but since we ASSERT(0), it is not the big issue... */ );
                         /* Restart the loop on wakeup */
                         goto again;
@@ -207 +207 @@
         } while (1);
         
-        TRACE_DEBUG(INFO, "An error occurred in session module! Expiry thread is terminating...");
+        TRACE_DEBUG(INFO, "A system error occurred in session module! Expiry thread is terminating...");
         ASSERT(0);
         return NULL;
@@ -442 +442 @@
                 /* We added a new expiring element, we must signal */
                 if (li == &exp_sentinel) {
-                        CHECK_POSIX( pthread_cond_signal(&exp_cond) );
+                        CHECK_POSIX_DO( pthread_cond_signal(&exp_cond), { ASSERT(0); } ); /* if it fails, we might not pop the cleanup handlers, but this should not happen -- and we'd have a serious problem otherwise */
                 }
                 
@@ -458 +458 @@
                 /* We're done */
                 pthread_cleanup_pop(0);
-                CHECK_POSIX( pthread_mutex_unlock( &exp_lock ) );
+                CHECK_POSIX_DO( pthread_mutex_unlock( &exp_lock ), { ASSERT(0); } ); /* if it fails, we might not pop the cleanup handler, but this should not happen -- and we'd have a serious problem otherwise */
         }
         
@@ -540 +540 @@
         /* We added a new expiring element, we must signal if it was in first position */
         if (session->expire.prev == &exp_sentinel) {
-                CHECK_POSIX( pthread_cond_signal(&exp_cond) );
+                CHECK_POSIX_DO( pthread_cond_signal(&exp_cond), { ASSERT(0); /* so that we don't have a pending cancellation handler */ } );
         }
 
@@ -573 +573 @@
         
         /* Unlink and invalidate */
-        CHECK_FCT( pthread_mutex_lock( H_LOCK(sess->hash) ) );
+        CHECK_POSIX( pthread_mutex_lock( H_LOCK(sess->hash) ) );
         pthread_cleanup_push( fd_cleanup_mutex, H_LOCK(sess->hash) );
-        CHECK_FCT( pthread_mutex_lock( &exp_lock ) );
+        CHECK_POSIX_DO( pthread_mutex_lock( &exp_lock ), { ASSERT(0); /* otherwise cleanup handler is not pop'd */ } );
         fd_list_unlink( &sess->chain_h );
         fd_list_unlink( &sess->expire ); /* no need to signal the condition here */
         sess->eyec = 0xdead;
-        CHECK_FCT( pthread_mutex_unlock( &exp_lock ) );
+        CHECK_POSIX_DO( pthread_mutex_unlock( &exp_lock ), { ASSERT(0); /* otherwise cleanup handler is not pop'd */ } );
         pthread_cleanup_pop(0);
-        CHECK_FCT( pthread_mutex_unlock( H_LOCK(sess->hash) ) );
+        CHECK_POSIX( pthread_mutex_unlock( H_LOCK(sess->hash) ) );
         
         /* Now destroy all states associated -- we don't take the lock since nobody can access this session anymore (in theory) */
@@ -614 +614 @@
         CHECK_POSIX( pthread_mutex_lock( H_LOCK(sess->hash) ) );
         pthread_cleanup_push( fd_cleanup_mutex, H_LOCK(sess->hash) );
-        CHECK_POSIX( pthread_mutex_lock( &exp_lock ) );
+        CHECK_POSIX_DO( pthread_mutex_lock( &exp_lock ), { ASSERT(0); /* otherwise, cleanup not poped on FreeBSD */ } );
         if (FD_IS_LIST_EMPTY(&sess->states)) {
                 fd_list_unlink( &sess->chain_h );
@@ -622 +622 @@
                 free(sess);
         }
-        CHECK_POSIX( pthread_mutex_unlock( &exp_lock ) );
+        CHECK_POSIX_DO( pthread_mutex_unlock( &exp_lock ), { ASSERT(0); /* otherwise, cleanup not poped on FreeBSD */ } );
         pthread_cleanup_pop(0);
         CHECK_POSIX( pthread_mutex_unlock( H_LOCK(hash) ) );
@@ -635 +635 @@
         struct fd_list * li;
         int already = 0;
+        int ret = 0;
         
         TRACE_ENTRY("%p %p %p", handler, session, state);
@@ -644 +645 @@
                         
         /* Create the new state object */
-        CHECK_MALLOC(new = malloc(sizeof(struct state)) );
+        CHECK_MALLOC_DO(new = malloc(sizeof(struct state)), { ret = ENOMEM; goto out; } );
         memset(new, 0, sizeof(struct state));
         
@@ -661 +662 @@
                 if (st->hdl->id == handler->id) {
                         TRACE_DEBUG(INFO, "A state was already stored for session '%s' and handler '%p', at location %p", session->sid, st->hdl, st->state);
-                        already = 1;
+                        already = EALREADY;
                 }
                 
@@ -673 +674 @@
                 free(new);
         }
-        
+out:
+        ;       
         pthread_cleanup_pop(0);
         CHECK_POSIX( pthread_mutex_unlock(&session->stlock) );
         
-        return already ? EALREADY : 0;
+        return ret ?: already;
 }