Summary:	ASTERISK-03358: [patch] - res_config_mysql updates
Reporter:	drmac (drmac)	Labels:
Date Opened:	2005-01-26 15:57:13.000-0600	Date Closed:	2005-01-27 19:25:45.000-0600
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Core/Configuration
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) res_config_mysql_1.5.1.patch ( 1) res_config_mysql_1.5.patch
Description:	- brings res_config_mysql.c into sync with 3406. - removed some dumb comments I had in. - added (what I believe is) better login flow/error checking on mysql_store_result - (per user request) added v1.5 comments - updated copyright
Comments:	By: drmac (drmac) 2005-01-26 15:58:36.000-0600 son of a... that should read "better logic flow" not login.. By: drmac (drmac) 2005-01-26 16:02:50.000-0600 good grief... disclaimer on file..shame there isn't a (+d) next to my name. By: Kevin P. Fleming (kpfleming) 2005-01-26 16:46:53.000-0600 I haven't checked the code in detail, but looking over the patch it looks OK. However, I'm concerned about the calls to ast_mutex_lock that are checking its return value; as it stands today, ast_mutex_lock will never fail, unless you are trying to lock an invalid (uninitialized) lock. If the lock is valid, you will eventually get it, but you may sleep for some time before you do. If what you want is to _try_ to get the lock and take alternative action if you cannot, then you need to use ast_mutex_trylock, which will not wait for the lock; you either get it, or you get a failure result back. By: drmac (drmac) 2005-01-26 17:45:52.000-0600 In the case of the usecount function, you are correct. It should try and get a lock; if it can't immediatly return 1 instead of waiting for the lock. In the case of the unload function, the current thread needs to wait for the lock so it knows it has total control of the object so it can unlock it before unloading the module. In the case of the reload function (similar to unload), if you issue the reload command, then you "want" it to reload. If you've got tons of mysql RealTime stuff going on, then the admin will never be able to find a "gap" to issue that command. Seems better (IMHO) to issue the command, wait for control, reload the module, then resume normal operations. So in actuality, this function (and unload) should NOT be checking the return value at all. They both should wait for the lock, then unlock, then re/unload; right? By: drmac (drmac) 2005-01-26 17:50:15.000-0600 In the unload function, the ast_mutex_lock should be moved to the top of the function right? That way you wait for the lock (ie: wait for other threads to finish) before closing the connection and unregistering all the functions. BTW: the ie comment isn't meant as any kind of insult or anything, I know that you know what you are talking about. Its more of a "im going to explain it back to you as I understand it and you tell me if what I told you is wrong." By: drmac (drmac) 2005-01-26 18:01:00.000-0600 don't apply 1.5.1 on top of 1.5 Delete your res_config_mysql.c, get a fresh copy from CVS and then apply 1.5.1 I didn't want people to have to patch a patch. edited on: 01-26-05 18:01 By: Kevin P. Fleming (kpfleming) 2005-01-26 18:06:28.000-0600 Yes, your comments sounds like the proper way to handle things. The only other thing you could do is to store a 'reload needed' flag that is set when the user sends the reload command, and have the reload happen some time later when the connection(s) are free... but the way you have it now is certainly usable. By: Mark Spencer (markster) 2005-01-26 20:53:30.000-0600 Fixed in CVS head, thanks!