Bugs item #3039943, was opened at 2010-08-05 12:52
Message generated for change (Comment added) made by viraptor
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=1086410&aid=3039943&group_id=232389
Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: modules
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Stanislaw Pitucha (viraptor)
Assigned to: Nobody/Anonymous (nobody)
Summary: db_mysql multiple issues in run_mysql_cmd wrapper
Initial Comment:
There seem to be a couple of issues hidden in `run_mysql_cmd` macro:
- if there is no connection, first `if` causes a fall through to checking
`mysql_errno()`, which has the return value of the last command, while it
assumes checking the (_cmd)'s return code
- the macro is used for both prepared statement and other commands, even though
mysql_stmt_* commands report the code via mysql_stmt_errno(), not mysql_errno()
- comparing mysql_errno to error codes and then getting mysql_error when error
code==0 seems like a stupid thing to do, unless it's a workaround for some
libmysqlclient bug
- "disconect" should be spelled "disconnect" :)
- reported errors are not true in functions using the wrapper - for example
re_init_statement reports `if (code<0) { LM_ERR("failed while
mysql_stmt_prepare()\n"); ...` while it can fail on db_mysql_connect which also
returns the result via the 'code' variable
- if the database disconnectts during mysql_stmt_reppare, statements are going
to be reset (reset_all_statements()), the final value of code will be that of
db_mysql_connect (probably 0) and ctx->stmt will not be initialised, but
mysql_stmt_close() will be run on it, causing a crash
- probably some others I haven't noticed...
Proposed solution - kill the macro and rewrite it in a sane way - I tried
fixing the separate problems, but it seems like there are too many issues with
it.
----------------------------------------------------------------------
>Comment By: Stanislaw Pitucha (viraptor)
Date: 2010-08-06 13:48
Message:
Uploaded my patch queue. It removes all occurrences of run_mysql_cmd and
replaces them with some common code. (they can be macro'd if someone really
wants to do that)
>From the limited testing I could do, it seem the patches work just fine.
To simulate the disconnect conditions, you can setup haproxy with very low
idle client disconnection time in front of the database itself.
----------------------------------------------------------------------
Comment By: Stanislaw Pitucha (viraptor)
Date: 2010-08-05 18:57
Message:
Attached a proposed way to change the code. It's only a couple of lines
longer in place of the macro, but the logic is more explicit and the
function-specific wrappers will remove problems related to error checking
the return codes. Tested on a specifically prepared mysql which drops the
connections tcp quite often - doesn't seem to cause crashes anymore.
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=1086410&aid=3039943&group_id=232389
_______________________________________________
Devel mailing list
Devel@lists.opensips.org
http://lists.opensips.org/cgi-bin/mailman/listinfo/devel
