Summary:	ASTERISK-11771: [patch] Race condition in manager interface.
Reporter:	Andriy I Pylypenko (bamby)	Labels:
Date Opened:	2008-04-02 08:48:09	Date Closed:	2008-04-02 16:09:33
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Core/ManagerInterface
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) manager_race_condition.diff
Description:	I use the manager interface (not HTTP) to identify the state of the channel and state changes are to be detected as soon as possible. However I experienced strange random delays that could last for several seconds especially if the system had small load. I've made investigation of the issue and found a cause of the problem. An events get reported via manager_event() function which appends the event to event queue and tries to inform the manager session thread about new event. Notification is done via pthread_kill(s->waiting_thread, SIGURG) call in order to interrupt the poll() call in get_input() function. But the problem is that the waiting_thread field has valid value only during the poll() call. Immediately after the poll() the waiting_thread is cleared and stays NULL until next poll(). So if an event comes at the instant when waiting_thread is AST_PTHREAD_NULL then this event has a good chance to stay unnoticed until next event or input from client. I've added a field to the struct mansession that act as a pending event indicator when the waiting_thread is NULL. This works pretty well for me.
Comments:	By: Mark Michelson (mmichelson) 2008-04-02 11:52:27 Good find, bamby. I took a look at your patch and it makes good sense to me. I'll get this committed ASAP. By: Digium Subversion (svnbot) 2008-04-02 12:31:22 Repository: asterisk Revision: 112468 U   branches/1.4/main/manager.c ------------------------------------------------------------------------ r112468 | mmichelson | 2008-04-02 12:31:20 -0500 (Wed, 02 Apr 2008) | 13 lines Fix a race condition in the manager. It is possible that a new manager event could be appended during a brief time when the manager is not waiting for input. If an event comes during this period, we need to set an indicator that there is an event pending so that the manager doesn't attempt to wait forever for an event that already happened. (closes issue ASTERISK-11771) Reported by: bamby Patches:      manager_race_condition.diff uploaded by bamby (license 430)  (comments added by me) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=112468 By: Digium Subversion (svnbot) 2008-04-02 12:32:18 Repository: asterisk Revision: 112469 _U  trunk/ U   trunk/main/manager.c ------------------------------------------------------------------------ r112469 | mmichelson | 2008-04-02 12:32:17 -0500 (Wed, 02 Apr 2008) | 21 lines Merged revisions 112468 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r112468 | mmichelson | 2008-04-02 12:36:04 -0500 (Wed, 02 Apr 2008) | 13 lines Fix a race condition in the manager. It is possible that a new manager event could be appended during a brief time when the manager is not waiting for input. If an event comes during this period, we need to set an indicator that there is an event pending so that the manager doesn't attempt to wait forever for an event that already happened. (closes issue ASTERISK-11771) Reported by: bamby Patches:      manager_race_condition.diff uploaded by bamby (license 430)  (comments added by me) ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=112469 By: Digium Subversion (svnbot) 2008-04-02 12:33:11 Repository: asterisk Revision: 112470 _U  branches/1.6.0/ U   branches/1.6.0/main/manager.c ------------------------------------------------------------------------ r112470 | mmichelson | 2008-04-02 12:33:11 -0500 (Wed, 02 Apr 2008) | 29 lines Merged revisions 112469 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r112469 | mmichelson | 2008-04-02 12:36:49 -0500 (Wed, 02 Apr 2008) | 21 lines Merged revisions 112468 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r112468 | mmichelson | 2008-04-02 12:36:04 -0500 (Wed, 02 Apr 2008) | 13 lines Fix a race condition in the manager. It is possible that a new manager event could be appended during a brief time when the manager is not waiting for input. If an event comes during this period, we need to set an indicator that there is an event pending so that the manager doesn't attempt to wait forever for an event that already happened. (closes issue ASTERISK-11771) Reported by: bamby Patches:      manager_race_condition.diff uploaded by bamby (license 430)  (comments added by me) ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=112470 By: Digium Subversion (svnbot) 2008-04-02 16:09:33 Repository: asterisk Revision: 112555 U   team/group/manager2/apps/app_queue.c U   team/group/manager2/main/event.c U   team/group/manager2/res/res_manager2.c ------------------------------------------------------------------------ r112555 | mmichelson | 2008-04-02 16:09:21 -0500 (Wed, 02 Apr 2008) | 29 lines Merged revisions 112469 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r112469 | mmichelson | 2008-04-02 12:36:49 -0500 (Wed, 02 Apr 2008) | 21 lines Merged revisions 112468 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r112468 | mmichelson | 2008-04-02 12:36:04 -0500 (Wed, 02 Apr 2008) | 13 lines Fix a race condition in the manager. It is possible that a new manager event could be appended during a brief time when the manager is not waiting for input. If an event comes during this period, we need to set an indicator that there is an event pending so that the manager doesn't attempt to wait forever for an event that already happened. (closes issue ASTERISK-11771) Reported by: bamby Patches:      manager_race_condition.diff uploaded by bamby (license 430)  (comments added by me) ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=112555