Review Board 1.7.16


Resolve some race conditions in chan_iax2 scheduler handling

Review Request #131 - Created Jan. 25, 2009 and submitted

Russell Bryant
/branches/1.4
Reviewers
asterisk-dev
Asterisk
The main problem with the scheduler handling was that there was improper synchronization between adding new scheduler entries and the handling of calculating how much time until the next task and sleeping for that long.  This patch adjusts the handling of the scheduler lock and condition to avoid the potential for a new scheduler item being added right before the scheduler thread goes to sleep.

The old code minimized the impact of this problem by waking up at least once per second to see if it had work to do.  Now, the code will not wake up any more often than it has to.

This patch is similar to http://reviewboard.digium.com/r/129/.  However, this patch is for 1.4, and is implemented in the least invasive way possible.
It compiles and looks right.  :-)

Diff revision 1 (Latest)

  1. /branches/1.4/channels/chan_iax2.c: Loading...
/branches/1.4/channels/chan_iax2.c
Revision 171119 New Change
[20] 983 lines
[+20] [+] static int __schedule_action(void (*func)(const void *data), const void *data, const char *funcname)
984

    
   
984

   
985
static int iax2_sched_add(struct sched_context *con, int when, ast_sched_cb callback, const void *data)
985
static int iax2_sched_add(struct sched_context *con, int when, ast_sched_cb callback, const void *data)
986
{
986
{
987
	int res;
987
	int res;
988

    
   
988

   

    
   
989
	ast_mutex_lock(&sched_lock);
989
	res = ast_sched_add(con, when, callback, data);
990
	res = ast_sched_add(con, when, callback, data);
990
	signal_condition(&sched_lock, &sched_cond);
991
	ast_cond_signal(&sched_cond);

    
   
992
	ast_mutex_unlock(&sched_lock);
991

    
   
993

   
992
	return res;
994
	return res;
993
}
995
}
994

    
   
996

   
995
static int send_ping(const void *data);
997
static int send_ping(const void *data);
[+20] [20] 2469 lines
[+20] [+] static int iax2_hangup(struct ast_channel *c)
3465
		if (iaxs[callno] && alreadygone) {
3467
		if (iaxs[callno] && alreadygone) {
3466
			if (option_debug)
3468
			if (option_debug)
3467
				ast_log(LOG_DEBUG, "Really destroying %s now...\n", c->name);
3469
				ast_log(LOG_DEBUG, "Really destroying %s now...\n", c->name);
3468
			iax2_destroy(callno);
3470
			iax2_destroy(callno);
3469
		} else if (iaxs[callno]) {
3471
		} else if (iaxs[callno]) {
3470
			ast_sched_add(sched, 10000, scheduled_destroy, CALLNO_TO_PTR(callno));
3472
			iax2_sched_add(sched, 10000, scheduled_destroy, CALLNO_TO_PTR(callno));
3471
		}
3473
		}
3472
	} else if (c->tech_pvt) {
3474
	} else if (c->tech_pvt) {
3473
		/* If this call no longer exists, but the channel still
3475
		/* If this call no longer exists, but the channel still
3474
		 * references it we need to set the channel's tech_pvt to null
3476
		 * references it we need to set the channel's tech_pvt to null
3475
		 * to avoid ast_channel_free() trying to free it.
3477
		 * to avoid ast_channel_free() trying to free it.
[+20] [20] 5685 lines
[+20] [+] static struct ast_channel *iax2_request(const char *type, int format, void *data, int *cause)
9161
	return c;
9163
	return c;
9162
}
9164
}
9163

    
   
9165

   
9164
static void *sched_thread(void *ignore)
9166
static void *sched_thread(void *ignore)
9165
{
9167
{
9166
	int count;
9168
	for (;;) {
9167
	int res;
9169
		int ms, count;
9168
	struct timeval tv;

   
9169
	struct timespec ts;
9170
		struct timespec ts;
9170

    
   
9171

   
9171
	for (;;) {

   
9172
		pthread_testcancel();
9172
		pthread_testcancel();

    
   
9173

   
9173
		ast_mutex_lock(&sched_lock);
9174
		ast_mutex_lock(&sched_lock);
9174
		res = ast_sched_wait(sched);
9175

   
9175
		if ((res > 1000) || (res < 0))
9176
		ms = ast_sched_wait(sched);
9176
			res = 1000;
9177

   
9177
		tv = ast_tvadd(ast_tvnow(), ast_samp2tv(res, 1000));
9178
		if (ms == -1) {

    
   
9179
			ast_cond_wait(&sched_cond, &sched_lock);

    
   
9180
		} else {

    
   
9181
			struct timeval tv;

    
   
9182
			tv = ast_tvadd(ast_tvnow(), ast_samp2tv(ms, 1000));
9178
		ts.tv_sec = tv.tv_sec;
9183
			ts.tv_sec = tv.tv_sec;
9179
		ts.tv_nsec = tv.tv_usec * 1000;
9184
			ts.tv_nsec = tv.tv_usec * 1000;
9180
		ast_cond_timedwait(&sched_cond, &sched_lock, &ts);
9185
			ast_cond_timedwait(&sched_cond, &sched_lock, &ts);

    
   
9186
		}

    
   
9187

   
9181
		ast_mutex_unlock(&sched_lock);
9188
		ast_mutex_unlock(&sched_lock);

    
   
9189

   
9182
		pthread_testcancel();
9190
		pthread_testcancel();
9183

    
   
9191

   
9184
		count = ast_sched_runq(sched);
9192
		count = ast_sched_runq(sched);
9185
		if (option_debug && count >= 20)
9193
		if (option_debug && count >= 20) {
9186
			ast_log(LOG_DEBUG, "chan_iax2: ast_sched_runq ran %d scheduled tasks all at once\n", count);
9194
			ast_log(LOG_DEBUG, "chan_iax2: ast_sched_runq ran %d scheduled tasks all at once\n", count);
9187
	}
9195
		}

    
   
9196
	}

    
   
9197

   
9188
	return NULL;
9198
	return NULL;
9189
}
9199
}
9190

    
   
9200

   
9191
static void *network_thread(void *ignore)
9201
static void *network_thread(void *ignore)
9192
{
9202
{
[+20] [20] 2145 lines
  1. /branches/1.4/channels/chan_iax2.c: Loading...

https://reviewboard.asterisk.org/ runs on a server provided by Digium, Inc. and uses bandwidth donated to the open source Asterisk community by API Digital Communications in Huntsville, AL USA.
Please report problems with this site to asteriskteam@digium.com.