bug_fixed: statistics (they were previously updated in an

unprotected way; hash entry locks were mistakenly used to
protect access to global stats structures; now each process
maintains its own stats, and fifo_stats sums all of them)

modules/tm/h_table.c

@@ -254,7 +254,7 @@ void free_hash_table(  )
 				free_cell( p_cell );
 			}
 		}
-
+		shm_free(tm_table);
 	}
 }
 
@@ -323,12 +323,7 @@ void insert_into_hash_table_unsafe( struct cell * p_cell )
 	/* update stats */
 	p_entry->cur_entries++;
 	p_entry->acc_entries++;
-	cur_stats->transactions++;
-	acc_stats->transactions++;
-	if (p_cell->local) {
-		cur_stats->client_transactions++;
-		acc_stats->client_transactions++;
-	}
+	t_stats_new(p_cell->local);
 }
 
 
@@ -369,15 +364,7 @@ void remove_from_hash_table_unsafe( struct cell * p_cell)
 	}
 #	endif
 	p_entry->cur_entries--;
-#	ifdef EXTRA_DEBUG
-	if (cur_stats->transactions==0) {
-		LOG(L_CRIT, "BUG: bad things happened: cur->transactions=0\n");
-		abort();
-	}
-#	endif
-	cur_stats->transactions--;
-	if (p_cell->local) cur_stats->client_transactions--;
-	cur_stats->waiting--;
+	t_stats_deleted(p_cell->local);
 
 	/* unlock( &(p_entry->mutex) ); */
 }

modules/tm/lock.c

@@ -310,6 +310,7 @@ error:
 void lock_cleanup()
 {
 	/* must check if someone uses them, for now just leave them allocated*/
+	if (timer_group_lock) shm_free(timer_group_lock);
 }
 
 #else
@@ -354,6 +355,7 @@ void lock_cleanup()
 	wait_semaphore = 0;
 #endif
 
+	if (timer_group_lock) shm_free(timer_group_lock);
 
 }
 #endif /*FAST_LOCK*/

modules/tm/t_funcs.c

@@ -39,6 +39,7 @@
 #include "t_fwd.h"
 #include "t_lookup.h"
 #include "config.h"
+#include "t_stats.h"
 
 
 /* ----------------------------------------------------- */
@@ -79,6 +80,7 @@ void tm_shutdown()
 	free_timer_table();
 	DBG("DEBUG: tm_shutdown : removing semaphores\n");
 	lock_cleanup();
+	free_tm_stats();
 	DBG("DEBUG: tm_shutdown : done\n");
 }
 

modules/tm/t_reply.c

@@ -74,20 +74,6 @@ unsigned int get_on_negative()
 	return goto_on_negative;
 }
 
-static void update_reply_stats( int code ) {
-	if (code>=600) {
-		acc_stats->completed_6xx++;
-	} else if (code>=500) {
-		acc_stats->completed_5xx++;
-	} else if (code>=400) {
-		acc_stats->completed_4xx++;
-	} else if (code>=300) {
-		acc_stats->completed_3xx++;
-	} else if (code>=200) {
-		acc_stats->completed_2xx++;
-	}
-}
-
 static char *build_ack(struct sip_msg* rpl,struct cell *trans,int branch,
 	unsigned int *ret_len)
 {
@@ -390,7 +376,7 @@ static int _reply( struct cell *trans, struct sip_msg* p_msg,
 	   on current transactions status */
 	/* t_update_timers_after_sending_reply( rb ); */
 	update_reply_stats( code );
-	acc_stats->replied_localy++;
+	tm_stats->replied_localy++;
 	if (lock) UNLOCK_REPLIES( trans );
 	
 	/* do UAC cleanup procedures in case we generated
@@ -544,6 +530,7 @@ enum rps relay_reply( struct cell *t, struct sip_msg *p_msg, int branch,
 	       or a stored message */
 		relayed_msg = branch==relay ? p_msg :  t->uac[relay].reply;
 		if (relayed_msg ==FAKED_REPLY) {
+			tm_stats->replied_localy++;
 			relayed_code = branch==relay
 				? msg_status : t->uac[relay].last_received;
 			buf = build_res_buf_from_sip_req( relayed_code,
@@ -674,6 +661,7 @@ enum rps local_reply( struct cell *t, struct sip_msg *p_msg, int branch,
 		winning_msg= branch==local_winner 
 			? p_msg :  t->uac[local_winner].reply;
 		if (winning_msg==FAKED_REPLY) {
+			tm_stats->replied_localy++;
 			winning_code = branch==local_winner
 				? msg_status : t->uac[local_winner].last_received;
 		} else {

modules/tm/t_stats.c

@@ -35,8 +35,9 @@
 #include "../../dprint.h"
 #include "../../config.h"
 #include "../../fifo_server.h"
+#include "../../pt.h"
 
-struct t_stats *cur_stats, *acc_stats;
+struct t_stats *tm_stats;
 
 
 /* we don't worry about locking data during reads (unlike
@@ -44,34 +45,37 @@ struct t_stats *cur_stats, *acc_stats;
   
 int print_stats(  FILE *f )
 {
-	fprintf(f, "Current:\n");
-	fprintf(f, "# of transactions: %10lu, ", 
-		cur_stats->transactions );
-	fprintf(f, "local: %10lu, ",
-		cur_stats->client_transactions );
-	fprintf(f, "waiting: %10lu" CLEANUP_EOL ,
-		cur_stats->waiting );
-
-	fprintf(f, "Total:\n");
-	fprintf(f, "# of transactions: %10lu,",
-		acc_stats->transactions );
-	fprintf(f, " local: %10lu,",
-		acc_stats->client_transactions );
-	fprintf(f, " waiting: %10lu" CLEANUP_EOL ,
-		acc_stats->waiting );
-
-	fprintf(f, "Replied localy: %10lu" CLEANUP_EOL ,
-		acc_stats->replied_localy );
-	fprintf(f, "Completion status 6xx: %10lu,",
-		acc_stats->completed_6xx );
-	fprintf(f, " 5xx: %10lu,",
-		acc_stats->completed_5xx );
-	fprintf(f, " 4xx: %10lu,",
-		acc_stats->completed_4xx );
-	fprintf(f, " 3xx: %10lu,",
-		acc_stats->completed_3xx );
-	fprintf(f, "2xx: %10lu" CLEANUP_EOL ,
-		acc_stats->completed_2xx );
+	unsigned long total, current, waiting, total_local;
+	int i;
+	int pno;
+
+	pno=process_count();
+	for(i=0, total=0, waiting=0, total_local=0; i<pno;i++) {
+		total+=tm_stats->s_transactions[i];
+		waiting+=tm_stats->s_waiting[i];
+		total_local+=tm_stats->s_client_transactions[i];
+	}
+	current=total-tm_stats->deleted;
+	waiting-=tm_stats->deleted;
+
+	
+
+	fprintf(f, "Current: %lu (%lu waiting) "
+		"Total: %lu (%lu local) " CLEANUP_EOL,
+		current, waiting, total, total_local);
+
+	fprintf(f, "Replied localy: %lu" CLEANUP_EOL ,
+		tm_stats->replied_localy );
+	fprintf(f, "Completion status 6xx: %lu,",
+		tm_stats->completed_6xx );
+	fprintf(f, " 5xx: %lu,",
+		tm_stats->completed_5xx );
+	fprintf(f, " 4xx: %lu,",
+		tm_stats->completed_4xx );
+	fprintf(f, " 3xx: %lu,",
+		tm_stats->completed_3xx );
+	fprintf(f, "2xx: %lu" CLEANUP_EOL ,
+		tm_stats->completed_2xx );
 	
 	return 1;
 }
@@ -101,24 +105,60 @@ int static fifo_stats( FILE *pipe, char *response_file )
 
 int init_tm_stats(void)
 {
-	cur_stats=shm_malloc(sizeof(struct t_stats));
-	if (cur_stats==0) {
+	int size;
+
+	tm_stats=shm_malloc(sizeof(struct t_stats));
+	if (tm_stats==0) {
 		LOG(L_ERR, "ERROR: init_stats: no mem for stats\n");
-		return -1;
+		goto error0;
 	}
-	acc_stats=shm_malloc(sizeof(struct t_stats));
-	if (acc_stats==0) {
+	memset(tm_stats, 0, sizeof(struct t_stats) );
+
+	size=sizeof(int)*process_count();
+	tm_stats->s_waiting=shm_malloc(size);
+	if (tm_stats->s_waiting==0) {
 		LOG(L_ERR, "ERROR: init_stats: no mem for stats\n");
-		shm_free(cur_stats);
-		return -1;
+		goto error1;
 	}
+	memset(tm_stats->s_waiting, 0, size );
 
+	tm_stats->s_transactions=shm_malloc(size);
+	if (tm_stats->s_transactions==0) {
+		LOG(L_ERR, "ERROR: init_stats: no mem for stats\n");
+		goto error2;
+	}
+	memset(tm_stats->s_transactions, 0, size );
+
+	tm_stats->s_client_transactions=shm_malloc(size);
+	if (tm_stats->s_client_transactions==0) {
+		LOG(L_ERR, "ERROR: init_stats: no mem for stats\n");
+		goto error3;
+	}
+	memset(tm_stats->s_client_transactions, 0, size );
+		 
 	if (register_fifo_cmd(fifo_stats, "t_stats", 0)<0) {
 		LOG(L_CRIT, "cannot register fifo stats\n");
-		return -1;
+		goto error4;
 	}
 
-	memset(cur_stats, 0, sizeof(struct t_stats) );
-	memset(acc_stats, 0, sizeof(struct t_stats) );
 	return 1;
+
+error4:
+	shm_free(tm_stats->s_client_transactions);
+error3:
+	shm_free(tm_stats->s_transactions);
+error2:
+	shm_free(tm_stats->s_waiting);
+error1:
+	shm_free(tm_stats);
+error0:
+	return -1;
+}
+
+void free_tm_stats()
+{
+	shm_free(tm_stats->s_client_transactions);
+	shm_free(tm_stats->s_transactions);
+	shm_free(tm_stats->s_waiting);
+	shm_free(tm_stats);
 }

modules/tm/t_stats.h

@@ -31,22 +31,60 @@
 #ifndef _T_STATS_H
 #define _T_STATS_H
 
+#include "../../pt.h"
 
-extern struct t_stats *cur_stats, *acc_stats;
+
+extern struct t_stats *tm_stats;
 
 struct t_stats {
+	/* number of transactions in wait state */
+	unsigned long *s_waiting;
 	/* number of server transactions */
-	unsigned long transactions;
+	unsigned long *s_transactions;
 	/* number of UAC transactions (part of transactions) */
-	unsigned long client_transactions;
-	/* number of transactions in wait state */
-	unsigned long waiting;
+	unsigned long *s_client_transactions;
 	/* number of transactions which completed with this status */
 	unsigned long completed_3xx, completed_4xx, completed_5xx, 
 		completed_6xx, completed_2xx;
 	unsigned long replied_localy;
+	unsigned long deleted;
 };
 
+inline void static t_stats_new(int local)
+{
+	/* keep it in process's piece of shmem */
+	tm_stats->s_transactions[process_no]++;
+	if(local) tm_stats->s_client_transactions[process_no]++;
+}
+
+inline void static t_stats_wait()
+{
+	/* keep it in process's piece of shmem */
+	tm_stats->s_waiting[process_no]++;
+}
+
+inline void static t_stats_deleted( int local )
+{
+	/* no locking needed here -- only timer process deletes */
+	tm_stats->deleted++;
+}
+
+static void update_reply_stats( int code ) {
+	if (code>=600) {
+		tm_stats->completed_6xx++;
+	} else if (code>=500) {
+		tm_stats->completed_5xx++;
+	} else if (code>=400) {
+		tm_stats->completed_4xx++;
+	} else if (code>=300) {
+		tm_stats->completed_3xx++;
+	} else if (code>=200) {
+		tm_stats->completed_2xx++;
+	}
+}
+
+
 int init_tm_stats(void);
+void free_tm_stats();
 
 #endif

modules/tm/timer.c

@@ -746,9 +746,9 @@ void set_1timer( struct timer_link *new_tl, enum lists list_id )
 		   afford updating wait statistics; I admit its not nice
 		   but it greatly utilizes existing lock 
 		*/
-		cur_stats->waiting++;acc_stats->waiting++;
 	}
 	unlock(list->mutex);
+	t_stats_wait();
 }