[asterisk-commits] murf: branch murf/selway r150382 - in /team/murf/selway: ./ cdr/ channels/

Thu Oct 16 19:55:22 CDT 2008

Author: murf
Date: Thu Oct 16 19:55:21 2008
New Revision: 150382

URL: http://svn.digium.com/view/asterisk?view=rev&rev=150382
Log:
Merged revisions 150056,150124,150298,150301,150304 via svnmerge from 
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r150056 | murf | 2008-10-16 09:26:10 -0600 (Thu, 16 Oct 2008) | 50 lines

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.



........
r150124 | rmudgett | 2008-10-16 09:56:06 -0600 (Thu, 16 Oct 2008) | 1 line

Fix memory leak found by customer
........
r150298 | mmichelson | 2008-10-16 17:34:37 -0600 (Thu, 16 Oct 2008) | 10 lines

Don't try to call a dialplan function's read callback from
the manager's GetVar handler if an invalid channel has
been specified. Several dialplan functions, including
CHANNEL and SIP_HEADER, do not check for NULL-ness of
the channel being passed in.

(closes issue #13715)
Reported by: makoto


........
r150301 | mmichelson | 2008-10-16 17:35:07 -0600 (Thu, 16 Oct 2008) | 3 lines

And don't forget to return on the error condition


........
r150304 | mmichelson | 2008-10-16 17:40:54 -0600 (Thu, 16 Oct 2008) | 6 lines

Reverting changes from commits 150298 and 150301 since
I was mistakenly under the assumption that dialplan functions
*always* required that a channel be present. I need to go
home earlier, I think :)


........

Modified:
    team/murf/selway/   (props changed)
    team/murf/selway/cdr/cdr_odbc.c
    team/murf/selway/channels/chan_misdn.c

Propchange: team/murf/selway/
------------------------------------------------------------------------------
    automerge = yessir

Propchange: team/murf/selway/
------------------------------------------------------------------------------

--- svnmerge-integrated (original)
+++ svnmerge-integrated Thu Oct 16 19:55:21 2008
@@ -1,1 +1,1 @@
-/branches/1.4:1-149888
+/branches/1.4:1-150381

Modified: team/murf/selway/cdr/cdr_odbc.c
URL: http://svn.digium.com/view/asterisk/team/murf/selway/cdr/cdr_odbc.c?view=diff&rev=150382&r1=150381&r2=150382
==============================================================================
--- team/murf/selway/cdr/cdr_odbc.c (original)
+++ team/murf/selway/cdr/cdr_odbc.c Thu Oct 16 19:55:21 2008
@@ -122,7 +122,6 @@
 		res = odbc_init();
 		if (res < 0) {
 			odbc_disconnect();
-//			ast_mutex_unlock(&odbc_lock);
 			return;
 		}				
 	}
@@ -135,7 +134,6 @@
 		SQLFreeHandle(SQL_HANDLE_STMT, ODBC_stmt);
 		ODBC_stmt = SQL_NULL_HANDLE;
 		odbc_disconnect();
-//		ast_mutex_unlock(&odbc_lock);
 		return;
 	}
 
@@ -151,7 +149,6 @@
 		SQLFreeHandle(SQL_HANDLE_STMT, ODBC_stmt);
 		ODBC_stmt = SQL_NULL_HANDLE;
 		odbc_disconnect();
-//		ast_mutex_unlock(&odbc_lock);
 		return;
 	}
 

Modified: team/murf/selway/channels/chan_misdn.c
URL: http://svn.digium.com/view/asterisk/team/murf/selway/channels/chan_misdn.c?view=diff&rev=150382&r1=150381&r2=150382
==============================================================================
--- team/murf/selway/channels/chan_misdn.c (original)
+++ team/murf/selway/channels/chan_misdn.c Thu Oct 16 19:55:21 2008
@@ -5375,6 +5375,7 @@
 {
 	ast_mutex_destroy(&jb->mutexjb);
 	
+	free(jb->ok);
 	free(jb->samples);
 	free(jb);
 }


