This patch is relevant to:

ABE-1628 and RYM-150398 and AST-103 in internal Digium 
bug trackers.

These fixes address a really subtle memory corruption
problem that would happen in machines heavily loaded
in production environments. The corruption would
always take the form of the STMT object getting
nulled out and one of the unixODBC calls would
crash trying to access statement->connection.

It isn't fully proven yet, but the server has
now been running 2.5 days without appreciable
memory growth, or any gain of %cpu, and no 
crashes. Whether this is the problem or not
on that server, these fixes are still warranted.

As it turns out, **I** introduced these errors
unwittingly, when I corrected another crash earlier.
I had formed the build_query routine, and failed
to remove mutex_unlock calls in 3 places in the
transplanted code. These unlocks would only
happen in error situations, but unlocking the
mutex early set the code up for a catastrophic
failure, it appears. It would happen only once
every 100K-200K or more calls, under heavy load... 
but that is enough.

If another crash occurs, with the same MO, 
I'll come back and remove my confession from the log, and
we'll keep searching, but the fact that we
have Asterisk dying from an asynchronous
wiping of the STMT object, only on some connection
error, and that the server has lived for 2.5
days on this code without a crash, sure make
it look like this was the problem!

Also, in several points, Statement handles are
set to NULL after SQLFreeHandle. This was mainly
for insurance, to guarantee a crash. As it turns
out, the code does not appear to be attempting
to use these freed pointers.

Asterisk owes a debt of gratitude to Federico Alves
and Frediano Ziglio for their untiring efforts in
finding this bug, among others.




git-svn-id: http://svn.digium.com/svn/asterisk/branches/1.4@150056 f38db490-d61c-443f-a65b-d21fe96a405b

1 files changed, 12 insertions, 3 deletions

diff --git a/cdr/cdr_odbc.c b/cdr/cdr_odbc.c
index 289f3ae5b..da8327375 100644
--- a/cdr/cdr_odbc.c
+++ b/cdr/cdr_odbc.c
@@ -84,9 +84,12 @@ static SQLHSTMT	ODBC_stmt;			/* global ODBC Statement Handle */
 
 static void odbc_disconnect(void)
 {
+	ODBC_stmt = SQL_NULL_HANDLE;
 	SQLDisconnect(ODBC_con);
 	SQLFreeHandle(SQL_HANDLE_DBC, ODBC_con);
+	ODBC_con = SQL_NULL_HANDLE;
 	SQLFreeHandle(SQL_HANDLE_ENV, ODBC_env);
+	ODBC_env = SQL_NULL_HANDLE;
 	connected = 0;
 }
 
@@ -119,7 +122,6 @@ static void build_query(struct ast_cdr *cdr, char *timestr, int timesize)
 		res = odbc_init();
 		if (res < 0) {
 			odbc_disconnect();
-			ast_mutex_unlock(&odbc_lock);
 			return;
 		}				
 	}
@@ -130,8 +132,8 @@ static void build_query(struct ast_cdr *cdr, char *timestr, int timesize)
 		if (option_verbose > 10)
 			ast_verbose( VERBOSE_PREFIX_4 "cdr_odbc: Failure in AllocStatement %d\n", ODBC_res);
 		SQLFreeHandle(SQL_HANDLE_STMT, ODBC_stmt);
+		ODBC_stmt = SQL_NULL_HANDLE;
 		odbc_disconnect();
-		ast_mutex_unlock(&odbc_lock);
 		return;
 	}
 
@@ -145,8 +147,8 @@ static void build_query(struct ast_cdr *cdr, char *timestr, int timesize)
 		if (option_verbose > 10)
 			ast_verbose( VERBOSE_PREFIX_4 "cdr_odbc: Error in PREPARE %d\n", ODBC_res);
 		SQLFreeHandle(SQL_HANDLE_STMT, ODBC_stmt);
+		ODBC_stmt = SQL_NULL_HANDLE;
 		odbc_disconnect();
-		ast_mutex_unlock(&odbc_lock);
 		return;
 	}
 
@@ -197,6 +199,7 @@ static int odbc_log(struct ast_cdr *cdr)
 				if (option_verbose > 10)
 					ast_verbose( VERBOSE_PREFIX_4 "cdr_odbc: Trying Query again!\n");
 				SQLFreeHandle(SQL_HANDLE_STMT, ODBC_stmt);
+				ODBC_stmt = SQL_NULL_HANDLE;
 				build_query(cdr, timestr, sizeof(timestr)); /* what a waste. If we have to reconnect, we have to build a new query */
 				res = odbc_do_query();
 				if (res < 0) {
@@ -210,6 +213,7 @@ static int odbc_log(struct ast_cdr *cdr)
 			ast_verbose( VERBOSE_PREFIX_4 "cdr_odbc: Query FAILED Call not logged!\n");
 	}
 	SQLFreeHandle(SQL_HANDLE_STMT, ODBC_stmt);
+	ODBC_stmt = SQL_NULL_HANDLE;
 	ast_mutex_unlock(&odbc_lock);
 	return 0;
 }
@@ -221,6 +225,7 @@ static int odbc_unload_module(void)
 		if (option_verbose > 10)
 			ast_verbose( VERBOSE_PREFIX_4 "cdr_odbc: Disconnecting from %s\n", dsn);
 		SQLFreeHandle(SQL_HANDLE_STMT, ODBC_stmt);
+		ODBC_stmt = SQL_NULL_HANDLE;
 		odbc_disconnect();
 	}
 	if (dsn) {
@@ -414,6 +419,7 @@ static int odbc_init(void)
 			if (option_verbose > 10)
 				ast_verbose( VERBOSE_PREFIX_4 "cdr_odbc: Error SetEnv\n");
 			SQLFreeHandle(SQL_HANDLE_ENV, ODBC_env);
+			ODBC_env = SQL_NULL_HANDLE;
 			connected = 0;
 			return -1;
 		}
@@ -424,6 +430,7 @@ static int odbc_init(void)
 			if (option_verbose > 10)
 				ast_verbose( VERBOSE_PREFIX_4 "cdr_odbc: Error AllocHDB %d\n", ODBC_res);
 			SQLFreeHandle(SQL_HANDLE_ENV, ODBC_env);
+			ODBC_env = SQL_NULL_HANDLE;
 			connected = 0;
 			return -1;
 		}
@@ -438,7 +445,9 @@ static int odbc_init(void)
 		if (option_verbose > 10)
 			ast_verbose( VERBOSE_PREFIX_4 "cdr_odbc: Error SQLConnect %d\n", ODBC_res);
 		SQLFreeHandle(SQL_HANDLE_DBC, ODBC_con);
+		ODBC_con = SQL_NULL_HANDLE;
 		SQLFreeHandle(SQL_HANDLE_ENV, ODBC_env);
+		ODBC_env = SQL_NULL_HANDLE;
 		connected = 0;
 		return -1;
 	} else {