Summary:	ASTERISK-13406: Crash on failure to execute SQL
Reporter:	jeffery palmer (darren1713)	Labels:
Date Opened:	2009-01-20 12:58:39.000-0600	Date Closed:	2011-06-07 14:08:21
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Resources/res_odbc
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:
Description:	* crashes when there is a SQL execute error. I have attached a valgrind stack of the error. It is impossible to reproduce in gdb because this error causes corrupted memory, which destroys the stack in gdb. In any case, the unixODBC code is very difficult to get a heads up on, but I *think* that you cannot free the stmt if there was an error. I cannot find any sample code to confirm this though... Version is actually 1.4.22.1 ****** ADDITIONAL INFORMATION ****** [Jan 20 06:10:14] ERROR[26325]: res_config_odbc.c:395 update_odbc: Executing statement 'UPDATE sip_buddies SET fullcontact=?, ipaddr=?, port=?, regseconds=?, username=?, regserver=? WHERE name=?' [Jan 20 06:10:14] WARNING[26325]: res_odbc.c:113 ast_odbc_prepare_and_execute: SQL Execute error -1! Attempting a reconnect... ==26320== ==26320== Thread 5: ==26320== Invalid free() / delete / delete[] ==26320==    at 0x4022C9C: free (in /usr/lib/valgrind/x86-linux/vgpreload_memcheck.so) ==26320==    by 0x4CF25F6: (within /usr/lib/libtdsodbc.so.0.0.0) ==26320==    by 0x4CF265C: (within /usr/lib/libtdsodbc.so.0.0.0) ==26320==    by 0x4CEA481: (within /usr/lib/libtdsodbc.so.0.0.0) ==26320==    by 0x4CEA83D: SQLFreeHandle (in /usr/lib/libtdsodbc.so.0.0.0) ==26320==    by 0x46ED658: __SQLFreeHandle (SQLFreeHandle.c:435) ==26320==    by 0x46EDCE0: SQLFreeHandle (SQLFreeHandle.c:598) ==26320==    by 0x490B320: ast_odbc_prepare_and_execute (res_odbc.c:114) SQLFreeHandle(SQL_HANDLE_STMT, stmt); ==26320==    by 0x469266A: ??? (res_config_odbc.c:396) stmt = ast_odbc_prepare_and_execute(obj, custom_prepare, &cps); ==26320==    by 0x809A4B5: ast_update_realtime (config.c:1472) ==26320==    by 0x4DDEEA6: ??? (chan_sip.c:7946) ==26320==    by 0x4DDEF1B: ??? (chan_sip.c:7962) ==26320==  Address 0x59f7608 is 0 bytes after a block of size 0 free'd ==26320==    at 0x4022C9C: free (in /usr/lib/valgrind/x86-linux/vgpreload_memcheck.so) ==26320==    by 0x402324B: realloc (in /usr/lib/valgrind/x86-linux/vgpreload_memcheck.so) ==26320==    by 0x4CF4B92: (within /usr/lib/libtdsodbc.so.0.0.0) ==26320==    by 0x4CF073E: (within /usr/lib/libtdsodbc.so.0.0.0) ==26320==    by 0x4CEEA3F: (within /usr/lib/libtdsodbc.so.0.0.0) ==26320==    by 0x4CEECFA: (within /usr/lib/libtdsodbc.so.0.0.0) ==26320==    by 0x4CE5163: SQLExecute (in /usr/lib/libtdsodbc.so.0.0.0) ==26320==    by 0x46EBA3E: SQLExecute (SQLExecute.c:283) ==26320==    by 0x490B176: ast_odbc_prepare_and_execute (res_odbc.c:99) res = SQLExecute(stmt); ==26320==    by 0x469266A: ??? (res_config_odbc.c:396) ==26320==    by 0x809A4B5: ast_update_realtime (config.c:1472) ==26320==    by 0x4DDEEA6: ??? (chan_sip.c:7946)
Comments:	By: jeffery palmer (darren1713) 2009-01-20 13:38:51.000-0600 Actually the more I review this the more apparent the issues become. It is almost impossible to handle the variations of sql errors that can happen in the current code. Some invalidate the stmt, some invalidate the connection, some do neither and there is quite a lack of error handling anywhere in the connection/disconnection/execution process. I suggest 2 things, which I would appreciate feedback on. First, is since the odbc layer handles connection pooling, to create and destroy connections on every sql statement. This will add a tiny bit of overhead to the odbc library, but massively simplify the odbc functions in *. The odbc library will already have pooled connections waiting for * anyways. The second is to re-vamp the prepare_and_execute function with something more like http://www.unixodbc.org/doc/ProgrammerManual/Tutorial/resul.html What do you think? By: Tilghman Lesher (tilghman) 2009-01-22 00:00:36.000-0600 I think it's clear that you've stumbled upon a bug in FreeTDS and you should report this valgrind output to that project. By: Tilghman Lesher (tilghman) 2009-01-22 00:10:06.000-0600 BTW, if you're going to assert that the code is doing it incorrectly, then I expect you to point to specific ODBC guidelines (either MSDN ODBC or IBM ODBC online documents work great) that support your contention.  After reading the documents on Statement Handles on both of these pages, I find this unlikely, though it's always possible that I have missed something. By: jeffery palmer (darren1713) 2009-01-22 09:33:44.000-0600 Next problem is * will not compile with the stable version of FreeTDS (version 0.82) because libtds.so no longer exists after version 0.64. This has to do with the configure script and makefiles. I can gladly test this again against the stable version when it's working. By: jeffery palmer (darren1713) 2009-01-22 10:22:26.000-0600 Actually the new version of freetds only conflicts with cdr_tds which references the old shared library and functions. * will compile res_odbc and all other odbc functionality. By: Tilghman Lesher (tilghman) 2009-01-22 11:13:27.000-0600 That's why cdr_tds is getting a rewrite in the 1.6 series.  However, given that it would require a rewrite, it is unlikely to be changed for the 1.4 series. This issue is squarely in the court of FreeTDS, not Asterisk, so this issue will be closed. By: Tilghman Lesher (tilghman) 2009-01-22 11:14:25.000-0600 If you'd like to discuss this more, please use one of the mailing lists, as this is a bugtracker, not a discussion forum. By: jeffery palmer (darren1713) 2009-01-22 13:17:49.000-0600 You are absolutely correct that freetds-0.64 is the problem. It will crash * every time there is a failure to execute sql and then free'd. I have fixed this by using freetds-0.82 and unixODBC-2.2.14. I would suggest adding a dependency check and fail to build res_odbc if freetds = 0.64, although res_odbc could be using other drivers. There's no good way to check for this but almost all distro's are still on freetds-0.82. In any case, it's better than having a crashing, memory corrupted *. Thanks for your input. By: Tilghman Lesher (tilghman) 2009-01-22 15:28:12.000-0600 res_odbc isn't dependent on FreeTDS.