Mercurial > hg > freeDiameter

changeset 203:536b1dde8761

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Some cleanups in the cnxctx module
author Sebastien Decugis <sdecugis@nict.go.jp>
date Wed, 10 Feb 2010 17:47:39 +0900
parents b4380eb4174d
children 2465698b9f31
files extensions/test_app/ta_cli.c freeDiameter/cnxctx.c freeDiameter/cnxctx.h libfreeDiameter/sessions.c
diffstat 4 files changed, 133 insertions(+), 86 deletions(-) [+]
line wrap: on
line diff
--- a/extensions/test_app/ta_cli.c	Wed Feb 10 14:31:33 2010 +0900
+++ b/extensions/test_app/ta_cli.c	Wed Feb 10 17:47:39 2010 +0900
@@ -242,9 +242,9 @@
 
 void ta_cli_fini(void)
 {
-	(void) fd_sess_handler_destroy(&ta_cli_reg);
+	ta_sig_fini();
 	
-	ta_sig_fini();
+	(void) fd_sess_handler_destroy(&ta_cli_reg);
 	
 	return;
 };
--- a/freeDiameter/cnxctx.c	Wed Feb 10 14:31:33 2010 +0900
+++ b/freeDiameter/cnxctx.c	Wed Feb 10 17:47:39 2010 +0900
@@ -226,7 +226,7 @@
 		fd_log_debug("].\n");
 	}
 	
-	CHECK_MALLOC_DO( cli = fd_cnx_init(1), { shutdown(cli_sock, SHUT_RDWR); return NULL; } );
+	CHECK_MALLOC_DO( cli = fd_cnx_init(1), { shutdown(cli_sock, SHUT_RDWR); close(cli_sock); return NULL; } );
 	cli->cc_socket = cli_sock;
 	cli->cc_proto = serv->cc_proto;
 	
@@ -324,10 +324,6 @@
 	}
 	
 	return cnx;
-
-error:
-	fd_cnx_destroy(cnx);
-	return NULL;
 }
 
 /* Same for SCTP, accepts a list of remote addresses to connect to (see sctp_connectx for how they are used) */
@@ -348,7 +344,7 @@
 	CHECK_FCT_DO( fd_sctp_client( &sock, no_ip6, port, list ), return NULL );
 	
 	/* Once the socket is created successfuly, prepare the remaining of the cnx */
-	CHECK_MALLOC_DO( cnx = fd_cnx_init(1), { shutdown(sock, SHUT_RDWR); return NULL; } );
+	CHECK_MALLOC_DO( cnx = fd_cnx_init(1), { shutdown(sock, SHUT_RDWR); close(sock); return NULL; } );
 	
 	cnx->cc_socket = sock;
 	cnx->cc_proto  = IPPROTO_SCTP;
@@ -527,6 +523,10 @@
 		CHECK_SYS_DO(ret, /* continue */);
 	}
 	
+	/* Mark the error */
+	if (ret <= 0)
+		conn->cc_goterror=1;
+	
 	return ret;
 }
 
@@ -548,6 +548,10 @@
 		CHECK_SYS_DO(ret, /* continue */);
 	}
 	
+	/* Mark the error */
+	if (ret <= 0)
+		conn->cc_goterror=1;
+	
 	return ret;
 }
 
@@ -656,6 +660,7 @@
 	do {
 		CHECK_FCT_DO( fd_sctp_recvmeta(conn->cc_socket, NULL, &buf, &bufsz, &event, &conn->cc_closing), goto error );
 		if (event == FDEVP_CNX_ERROR) {
+			conn->cc_goterror = 1;
 			goto error;
 		}
 		
@@ -707,9 +712,47 @@
 			}
 		} );
 end:	
+	if (ret <= 0)
+		conn->cc_goterror = 1;
 	return ret;
 }
 
+/* Wrapper around gnutls_record_send to handle some error codes */
+static ssize_t fd_tls_send_handle_error(struct cnxctx * conn, gnutls_session_t session, void * data, size_t sz)
+{
+	ssize_t ret;
+again:	
+	CHECK_GNUTLS_DO( ret = gnutls_record_send(session, data, sz),
+		{
+			switch (ret) {
+				case GNUTLS_E_REHANDSHAKE: 
+					if (!conn->cc_closing)
+						CHECK_GNUTLS_DO( ret = gnutls_handshake(session),
+							{
+								if (TRACE_BOOL(INFO)) {
+									fd_log_debug("TLS re-handshake failed on socket %d (%s) : %s\n", conn->cc_socket, conn->cc_id, gnutls_strerror(ret));
+								}
+								goto end;
+							} );
+
+				case GNUTLS_E_AGAIN:
+				case GNUTLS_E_INTERRUPTED:
+					if (!conn->cc_closing)
+						goto again;
+					TRACE_DEBUG(INFO, "Connection is closing, so abord gnutls_record_send now.");
+					break;
+
+				default:
+					TRACE_DEBUG(INFO, "This TLS error is not handled, assume unrecoverable error");
+			}
+		} );
+end:	
+	if (ret <= 0)
+		conn->cc_goterror = 1;
+	return ret;
+}
+
+
 /* The function that receives TLS data and re-builds a Diameter message -- it exits only on error or cancelation */
 int fd_tls_rcvthr_core(struct cnxctx * conn, gnutls_session_t session)
 {
@@ -798,12 +841,12 @@
 	
 	CHECK_PARAMS( conn && Target_Queue(conn) && (!conn->cc_tls) && (!conn->cc_loop));
 	
+	/* Release resources in case of a previous call was already made */
+	CHECK_FCT_DO( fd_thr_term(&conn->cc_rcvthr), /* continue */);
+	
 	/* Save the loop request */
 	conn->cc_loop = loop;
 	
-	/* Release resources in case of a previous call was already made */
-	CHECK_FCT_DO( fd_thr_term(&conn->cc_rcvthr), /* continue */);
-	
 	switch (conn->cc_proto) {
 		case IPPROTO_TCP:
 			/* Start the tcp_notls thread */
@@ -1104,29 +1147,32 @@
 				if (TRACE_BOOL(INFO)) {
 					fd_log_debug("TLS Handshake failed on socket %d (%s) : %s\n", conn->cc_socket, conn->cc_id, gnutls_strerror(ret));
 				}
+				conn->cc_goterror = 1;
 				return EINVAL;
 			} );
 
-		/* Now verify the remote credentials are valid -- only simple test here */
-		CHECK_FCT( fd_tls_verify_credentials(conn->cc_tls_para.session, conn, 1) );
+		/* Now verify the remote credentials are valid -- only simple tests here */
+		CHECK_FCT_DO( fd_tls_verify_credentials(conn->cc_tls_para.session, conn, 1), 
+			{  
+				CHECK_GNUTLS_DO( gnutls_bye(conn->cc_tls_para.session, GNUTLS_SHUT_RDWR), /* Continue */ );
+				gnutls_deinit(conn->cc_tls_para.session);
+				return EINVAL;
+			});
 	}
 
+	/* Mark the connection as protected from here */
+	conn->cc_tls = 1;
+
 	/* Multi-stream TLS: handshake other streams as well */
 	if (conn->cc_sctp_para.pairs > 1) {
 #ifndef DISABLE_SCTP
 		/* Resume all additional sessions from the master one. */
 		CHECK_FCT(fd_sctps_handshake_others(conn, priority, alt_creds));
-		
-		/* Mark the connection as protected from here */
-		conn->cc_tls = 1;
 
 		/* Start decrypting the messages from all threads and queuing them in target queue */
 		CHECK_FCT(fd_sctps_startthreads(conn));
 #endif /* DISABLE_SCTP */
 	} else {
-		/* Mark the connection as protected from here */
-		conn->cc_tls = 1;
-
 		/* Start decrypting the data */
 		CHECK_POSIX( pthread_create( &conn->cc_rcvthr, NULL, rcvthr_tls_single, conn ) );
 	}
@@ -1210,37 +1256,6 @@
 	return 0;
 }
 
-/* Wrapper around gnutls_record_send to handle some error codes */
-static ssize_t fd_tls_send_handle_error(struct cnxctx * conn, gnutls_session_t session, void * data, size_t sz)
-{
-	ssize_t ret;
-again:	
-	CHECK_GNUTLS_DO( ret = gnutls_record_send(session, data, sz),
-		{
-			switch (ret) {
-				case GNUTLS_E_REHANDSHAKE: 
-					CHECK_GNUTLS_DO( ret = gnutls_handshake(session),
-						{
-							if (TRACE_BOOL(INFO)) {
-								fd_log_debug("TLS re-handshake failed on socket %d (%s) : %s\n", conn->cc_socket, conn->cc_id, gnutls_strerror(ret));
-							}
-							goto end;
-						} );
-
-				case GNUTLS_E_AGAIN:
-				case GNUTLS_E_INTERRUPTED:
-					goto again;
-
-				default:
-					TRACE_DEBUG(INFO, "This TLS error is not handled, assume unrecoverable error");
-			}
-		} );
-end:	
-	return ret;
-}
-
-
-
 /* Send function when no multi-stream is involved, or sending on stream #0 (send() always use stream 0)*/
 static int send_simple(struct cnxctx * conn, unsigned char * buf, size_t len)
 {
@@ -1263,7 +1278,7 @@
 {
 	TRACE_ENTRY("%p %p %zd", conn, buf, len);
 	
-	CHECK_PARAMS(conn && (conn->cc_socket > 0) && buf && len);
+	CHECK_PARAMS(conn && (conn->cc_socket > 0) && (! conn->cc_goterror) && buf && len);
 
 	TRACE_DEBUG(FULL, "Sending %zdb %sdata on connection %s", len, conn->cc_tls ? "TLS-protected ":"", conn->cc_id);
 	
@@ -1287,7 +1302,7 @@
 				CHECK_FCT( send_simple(conn, buf, len) );
 			} else {
 				if (!conn->cc_tls) {
-					CHECK_FCT( fd_sctp_sendstr(conn->cc_socket, conn->cc_sctp_para.next, buf, len, &conn->cc_closing) );
+					CHECK_FCT_DO( fd_sctp_sendstr(conn->cc_socket, conn->cc_sctp_para.next, buf, len, &conn->cc_closing), { conn->cc_goterror = 1; return ENOTCONN; } );
 				} else {
 					/* push the record to the appropriate session */
 					ssize_t ret;
@@ -1329,15 +1344,20 @@
 	if (conn->cc_tls) {
 #ifndef DISABLE_SCTP
 		if (conn->cc_sctp_para.pairs > 1) {
-			/* Bye on master session */
-			CHECK_GNUTLS_DO( gnutls_bye(conn->cc_tls_para.session, GNUTLS_SHUT_WR), /* Continue */ );
-			
-			/* and other stream pairs */
-			fd_sctps_bye(conn);
-			
-			/* Now wait for all decipher threads to terminate */
-			fd_sctps_waitthreadsterm(conn);
-			
+			if (! conn->cc_goterror ) {
+				/* Bye on master session */
+				CHECK_GNUTLS_DO( gnutls_bye(conn->cc_tls_para.session, GNUTLS_SHUT_WR), /* Continue */ );
+
+				/* and other stream pairs */
+				fd_sctps_bye(conn);
+
+				/* Now wait for all decipher threads to terminate */
+				fd_sctps_waitthreadsterm(conn);
+			} else {
+				/* Abord the threads, the connection is dead already */
+				fd_sctps_stopthreads(conn);
+			}
+
 			/* Deinit gnutls resources */
 			fd_sctps_gnutls_deinit_others(conn);
 			gnutls_deinit(conn->cc_tls_para.session);
@@ -1348,13 +1368,18 @@
 		} else {
 #endif /* DISABLE_SCTP */
 		/* We are not using the sctps wrapper layer */
-			/* Master session */
-			CHECK_GNUTLS_DO( gnutls_bye(conn->cc_tls_para.session, GNUTLS_SHUT_WR), /* Continue */ );
-		
-			/* In this case, just wait for thread rcvthr_tls_single to terminate */
-			if (conn->cc_rcvthr != (pthread_t)NULL) {
-				CHECK_POSIX_DO(  pthread_join(conn->cc_rcvthr, NULL), /* continue */  );
-				conn->cc_rcvthr = (pthread_t)NULL;
+			if (! conn->cc_goterror ) {
+				/* Master session */
+				CHECK_GNUTLS_DO( gnutls_bye(conn->cc_tls_para.session, GNUTLS_SHUT_WR), /* Continue */ );
+
+				/* In this case, just wait for thread rcvthr_tls_single to terminate */
+				if (conn->cc_rcvthr != (pthread_t)NULL) {
+					CHECK_POSIX_DO(  pthread_join(conn->cc_rcvthr, NULL), /* continue */  );
+					conn->cc_rcvthr = (pthread_t)NULL;
+				}
+			} else {
+				/* Cancel the receiver thread in case it did not already terminate */
+				CHECK_FCT_DO( fd_thr_term(&conn->cc_rcvthr), /* continue */ );
 			}
 			
 			/* Free the resources of the TLS session */
@@ -1365,7 +1390,7 @@
 #endif /* DISABLE_SCTP */
 	}
 	
-	/* Terminate the thread in case it is not done yet */
+	/* Terminate the thread in case it is not done yet -- is there any such case left ?*/
 	CHECK_FCT_DO( fd_thr_term(&conn->cc_rcvthr), /* continue */ );
 		
 	/* Shut the connection down */
--- a/freeDiameter/cnxctx.h	Wed Feb 10 14:31:33 2010 +0900
+++ b/freeDiameter/cnxctx.h	Wed Feb 10 17:47:39 2010 +0900
@@ -47,6 +47,7 @@
 
 	int 		cc_proto;	/* IPPROTO_TCP or IPPROTO_SCTP */
 	int		cc_tls;		/* Is TLS already started ? */
+	int		cc_goterror;	/* True when an error occurred on the socket */
 	int		cc_closing;	/* True if the object is being destroyed: we don't send events anymore */
 
 	pthread_t	cc_rcvthr;	/* thread for receiving messages on the connection */
--- a/libfreeDiameter/sessions.c	Wed Feb 10 14:31:33 2010 +0900
+++ b/libfreeDiameter/sessions.c	Wed Feb 10 17:47:39 2010 +0900
@@ -163,19 +163,20 @@
 	fd_log_threadname ( "Session/expire" );
 	TRACE_ENTRY( "" );
 	
-	CHECK_POSIX_DO( pthread_mutex_lock(&exp_lock),  goto error );
-	pthread_cleanup_push( fd_cleanup_mutex, &exp_lock );
 	
 	do {
 		struct timespec	now;
 		struct session * first;
 		
+		CHECK_POSIX_DO( pthread_mutex_lock(&exp_lock),  break );
+		pthread_cleanup_push( fd_cleanup_mutex, &exp_lock );
+again:		
 		/* Check if there are expiring sessions available */
 		if (FD_IS_LIST_EMPTY(&exp_sentinel)) {
 			/* Just wait for a change or cancelation */
-			CHECK_POSIX_DO( pthread_cond_wait( &exp_cond, &exp_lock ), goto error );
+			CHECK_POSIX_DO( pthread_cond_wait( &exp_cond, &exp_lock ), break );
 			/* Restart the loop on wakeup */
-			continue;
+			goto again;
 		}
 		
 		/* Get the pointer to the session that expires first */
@@ -183,28 +184,27 @@
 		ASSERT( VALIDATE_SI(first) );
 		
 		/* Get the current time */
-		CHECK_SYS_DO(  clock_gettime(CLOCK_REALTIME, &now),  goto error  );
+		CHECK_SYS_DO(  clock_gettime(CLOCK_REALTIME, &now),  break  );
 
 		/* If first session is not expired, we just wait until it happens */
 		if ( TS_IS_INFERIOR( &now, &first->timeout ) ) {
 			
 			CHECK_POSIX_DO2(  pthread_cond_timedwait( &exp_cond, &exp_lock, &first->timeout ),  
 					ETIMEDOUT, /* ETIMEDOUT is a normal error, continue */,
-					/* on other error, */ goto error );
+					/* on other error, */ break );
 	
 			/* on wakeup, loop */
-			continue;
+			goto again;
 		}
 		
 		/* Now, the first session in the list is expired; destroy it */
-		CHECK_POSIX_DO( pthread_mutex_unlock(&exp_lock),  goto error );
-		CHECK_FCT_DO( fd_sess_destroy( &first ), goto error );
-		CHECK_POSIX_DO( pthread_mutex_lock(&exp_lock),  goto error );
+		pthread_cleanup_pop( 0 );
+		CHECK_POSIX_DO( pthread_mutex_unlock(&exp_lock),  break );
+		
+		CHECK_FCT_DO( fd_sess_destroy( &first ), break );
 		
 	} while (1);
 	
-	pthread_cleanup_pop( 1 );
-error:
 	TRACE_DEBUG(INFO, "An error occurred in session module! Expiry thread is terminating...");
 	ASSERT(0);
 	return NULL;
@@ -346,6 +346,8 @@
 			sidlen = strlen(sid);
 		}
 	} else {
+		uint32_t sid_h_cpy;
+		uint32_t sid_l_cpy;
 		/* "<diamId>;<high32>;<low32>[;opt]" */
 		sidlen = strlen(diamId);
 		sidlen += 22; /* max size of ';<high32>;<low32>' */
@@ -353,20 +355,22 @@
 			sidlen += 1 + (optlen ?: strlen(opt)) ;
 		sidlen++; /* space for the final \0 also */
 		CHECK_MALLOC( sid = malloc(sidlen) );
+		
 		CHECK_POSIX( pthread_mutex_lock(&sid_lock) );
 		if ( ++sid_l == 0 ) /* overflow */
 			++sid_h;
+		sid_h_cpy = sid_h;
+		sid_l_cpy = sid_l;
+		CHECK_POSIX( pthread_mutex_unlock(&sid_lock) );
 		
 		if (opt) {
 			if (optlen)
-				sidlen = snprintf(sid, sidlen, "%s;%u;%u;%.*s", diamId, sid_h, sid_l, (int)optlen, opt);
+				sidlen = snprintf(sid, sidlen, "%s;%u;%u;%.*s", diamId, sid_h_cpy, sid_l_cpy, (int)optlen, opt);
 			else
-				sidlen = snprintf(sid, sidlen, "%s;%u;%u;%s", diamId, sid_h, sid_l, opt);
+				sidlen = snprintf(sid, sidlen, "%s;%u;%u;%s", diamId, sid_h_cpy, sid_l_cpy, opt);
 		} else {
-			sidlen = snprintf(sid, sidlen, "%s;%u;%u", diamId, sid_h, sid_l);
+			sidlen = snprintf(sid, sidlen, "%s;%u;%u", diamId, sid_h_cpy, sid_l_cpy);
 		}
-	
-		CHECK_POSIX( pthread_mutex_unlock(&sid_lock) );
 	}
 	
 	/* Initialize the session object now, to spend less time inside locked section later. 
@@ -375,6 +379,8 @@
 	
 	/* Now find the place to add this object in the hash table. */
 	CHECK_POSIX( pthread_mutex_lock( H_LOCK(sess->hash) ) );
+	pthread_cleanup_push( fd_cleanup_mutex, H_LOCK(sess->hash) );
+	
 	for (li = H_LIST(sess->hash)->next; li != H_LIST(sess->hash); li = li->next) {
 		int cmp;
 		struct session * s = (struct session *)(li->o);
@@ -403,6 +409,7 @@
 		
 		/* We must also insert in the expiry list */
 		CHECK_POSIX( pthread_mutex_lock( &exp_lock ) );
+		pthread_cleanup_push( fd_cleanup_mutex, &exp_lock );
 		
 		/* Find the position in that list. We take it in reverse order */
 		for (li = exp_sentinel.prev; li != &exp_sentinel; li = li->prev) {
@@ -429,9 +436,11 @@
 		#endif
 		
 		/* We're done */
+		pthread_cleanup_pop(0);
 		CHECK_POSIX( pthread_mutex_unlock( &exp_lock ) );
 	}
 	
+	pthread_cleanup_pop(0);
 	CHECK_POSIX( pthread_mutex_unlock( H_LOCK(sess->hash) ) );
 	
 	/* If a session already existed, we must destroy the new element */
@@ -491,6 +500,7 @@
 	
 	/* Lock -- do we need to lock the hash table as well? I don't think so... */
 	CHECK_POSIX( pthread_mutex_lock( &exp_lock ) );
+	pthread_cleanup_push( fd_cleanup_mutex, &exp_lock );
 	
 	/* Update the timeout */
 	fd_list_unlink(&session->expire);
@@ -524,6 +534,7 @@
 	#endif
 
 	/* We're done */
+	pthread_cleanup_pop(0);
 	CHECK_POSIX( pthread_mutex_unlock( &exp_lock ) );
 	
 	return 0;
@@ -542,11 +553,13 @@
 	
 	/* Unlink and invalidate */
 	CHECK_FCT( pthread_mutex_lock( H_LOCK(sess->hash) ) );
+	pthread_cleanup_push( fd_cleanup_mutex, H_LOCK(sess->hash) );
 	CHECK_FCT( pthread_mutex_lock( &exp_lock ) );
 	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 ) );
+	pthread_cleanup_pop(0);
 	CHECK_FCT( 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) */
@@ -577,6 +590,7 @@
 	*session = NULL;
 	
 	CHECK_FCT( pthread_mutex_lock( H_LOCK(sess->hash) ) );
+	pthread_cleanup_push( fd_cleanup_mutex, H_LOCK(sess->hash) );
 	CHECK_FCT( pthread_mutex_lock( &exp_lock ) );
 	if (FD_IS_LIST_EMPTY(&sess->states)) {
 		fd_list_unlink( &sess->chain_h );
@@ -586,6 +600,7 @@
 		free(sess);
 	}
 	CHECK_FCT( pthread_mutex_unlock( &exp_lock ) );
+	pthread_cleanup_pop(0);
 	CHECK_FCT( pthread_mutex_unlock( H_LOCK(sess->hash) ) );
 	
 	return 0;
@@ -603,6 +618,7 @@
 	
 	/* Lock the session state list */
 	CHECK_POSIX( pthread_mutex_lock(&session->stlock) );
+	pthread_cleanup_push( fd_cleanup_mutex, &session->stlock );
 			
 	/* Create the new state object */
 	CHECK_MALLOC(new = malloc(sizeof(struct state)) );
@@ -635,6 +651,7 @@
 		free(new);
 	}
 	
+	pthread_cleanup_pop(0);
 	CHECK_POSIX( pthread_mutex_unlock(&session->stlock) );
 	
 	return already ? EALREADY : 0;
@@ -653,6 +670,7 @@
 	
 	/* Lock the session state list */
 	CHECK_POSIX( pthread_mutex_lock(&session->stlock) );
+	pthread_cleanup_push( fd_cleanup_mutex, &session->stlock );
 	
 	/* find the state in the list */
 	for (li = session->states.next; li != &session->states; li = li->next) {
@@ -670,6 +688,7 @@
 		free(st);
 	}
 	
+	pthread_cleanup_pop(0);
 	CHECK_POSIX( pthread_mutex_unlock(&session->stlock) );
 	
 	return 0;
@@ -737,10 +756,12 @@
 		fd_log_debug("\t  %*s  timeout %s.%09ld\n", level, "", buf, session->timeout.tv_nsec);
 
 		CHECK_POSIX_DO( pthread_mutex_lock(&session->stlock), /* ignore */ );
+		pthread_cleanup_push( fd_cleanup_mutex, &session->stlock );
 		for (li = session->states.next; li != &session->states; li = li->next) {
 			struct state * st = (struct state *)(li->o);
 			fd_log_debug("\t  %*s    handler %d registered data %p\n", level, "", st->hdl->id, st->state);
 		}
+		pthread_cleanup_pop(0);
 		CHECK_POSIX_DO( pthread_mutex_unlock(&session->stlock), /* ignore */ );
 	}
 	fd_log_debug("\t  %*s -- end of session @%p --\n", level, "", session);
"Welcome to our mercurial repository"