Review Board 1.7.16


Fix directed group pickup feature code *8 with pickupsounds enabled , deadlock and segfault, affects 1.8.0 and trunk

Review Request #1185 - Created April 17, 2011 and submitted

Alec Davis
trunk
18654
Reviewers
asterisk-dev
rmudgett
Asterisk
Since 1.8, the new pickupsound and pickupfailsound in features.conf cause many issues.

1). chan_sip:handle_request_invite() shouldn't be playing out the fail/success audio, as it has 'netlock' locked.
2). dialplan applications for directed_pickups shouldn't beep.
3). feature code for directed pickup should beep on success/failure if configured.

Moved app_directed:pickup_do() to features:ast_do_pickup().

Functions below, all now use the new ast_do_pickup()
app_directed_pickup.c:
   pickup_by_channel()
   pickup_by_exten()
   pickup_by_mark()
   pickup_by_part()
features.c:
   ast_pickup_call()

Created a sip_pickup() thread to handle the pickup and playout the audio, spawned from handle_request_invite.
pickup using *8 feature code, with pickup sounds enabled/disabled

exten => 71,1,Pickup()           ; any ringing extension in same pickupgroup 
exten => 72,1,Pickup(85@phones)  ; dahdi extension
exten => 73,1,Pickup(823@phones) ; sip extension

Changes between revision 5 and 8

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16

  1. trunk/apps/app_directed_pickup.c: Loading...
  2. trunk/channels/chan_sip.c: Loading...
  3. trunk/include/asterisk/features.h: Loading...
  4. trunk/main/features.c: Loading...
  5. trunk/res/res_features.exports.in: Loading...
trunk/apps/app_directed_pickup.c
Diff Revision 5 Diff Revision 8
[20] 97 lines
[+20] [+] static const char app[] = "Pickup";
98
/*! \todo This application should return a result code, like PICKUPRESULT */
98
/*! \todo This application should return a result code, like PICKUPRESULT */
99

    
   
99

   
100
/* Helper function that determines whether a channel is capable of being picked up */
100
/* Helper function that determines whether a channel is capable of being picked up */
101
static int can_pickup(struct ast_channel *chan)
101
static int can_pickup(struct ast_channel *chan)
102
{
102
{
103
	if (!chan->pbx && (chan->_state == AST_STATE_RINGING || chan->_state == AST_STATE_RING || chan->_state == AST_STATE_DOWN))
103
	if (!chan->pbx && !chan->masq && (chan->_state == AST_STATE_RINGING || chan->_state == AST_STATE_RING || chan->_state == AST_STATE_DOWN))
104
		return 1;
104
		return 1;
105
	else
105
	else
106
		return 0;
106
		return 0;
107
}
107
}
108

    
   
108

   
[+20] [20] 57 lines
[+20] [+] static int pickup_by_channel(struct ast_channel *chan, char *pickup)
166

    
   
166

   
167
	/* Just check that we are not picking up the SAME as target */
167
	/* Just check that we are not picking up the SAME as target */
168
	if (chan != target) {
168
	if (chan != target) {
169
		res = ast_do_pickup(chan, target);
169
		res = ast_do_pickup(chan, target);
170
	}
170
	}
171

    
   

   
172
	ast_channel_unlock(target);
171
	ast_channel_unlock(target);
173
	target = ast_channel_unref(target);
172
	target = ast_channel_unref(target);
174

    
   
173

   
175
	return res;
174
	return res;
176
}
175
}
[+20] [20] 10 lines
[+20] [+] static int pickup_by_exten(struct ast_channel *chan, const char *exten, const char *context)
187
	}
186
	}
188

    
   
187

   
189
	while ((target = ast_channel_iterator_next(iter))) {
188
	while ((target = ast_channel_iterator_next(iter))) {
190
		ast_channel_lock(target);
189
		ast_channel_lock(target);
191
		if ((chan != target) && can_pickup(target)) {
190
		if ((chan != target) && can_pickup(target)) {

    
   
191
			ast_log(LOG_NOTICE,"%s pickup by %s\n", target->name, chan->name);
192
			break;
192
			break;
193
		}
193
		}
194
		ast_channel_unlock(target);
194
		ast_channel_unlock(target);
195
		target = ast_channel_unref(target);
195
		target = ast_channel_unref(target);
196
	}
196
	}
[+20] [20] 31 lines
[+20] [+] static int find_by_mark(void *obj, void *arg, void *data, int flags)
228
static int pickup_by_mark(struct ast_channel *chan, const char *mark)
228
static int pickup_by_mark(struct ast_channel *chan, const char *mark)
229
{
229
{
230
	struct ast_channel *target;
230
	struct ast_channel *target;
231
	int res = -1;
231
	int res = -1;
232

    
   
232

   
233
	if ((target = ast_channel_callback(find_by_mark, NULL, (char *) mark, 0))) {
233
	if (!(target = ast_channel_callback(find_by_mark, NULL, (char *) mark, 0))) {

    
   
234
		return res;

    
   
235
	}

    
   
236

   
234
		ast_channel_lock(target);
237
	ast_channel_lock(target);

    
   
238
	if (can_pickup(target)) {
235
		res = ast_do_pickup(chan, target);
239
		res = ast_do_pickup(chan, target);

    
   
240
	} else {

    
   
241
		ast_log(LOG_WARNING, "target has gone, or not ringing anymore for %s\n", chan->name);

    
   
242
	}
236
		ast_channel_unlock(target);
243
	ast_channel_unlock(target);
237
		target = ast_channel_unref(target);
244
	target = ast_channel_unref(target);
238
	}

   
239

    
   
245

   
240
	return res;
246
	return res;
241
}
247
}
242

    
   
248

   
243
static int find_channel_by_group(void *obj, void *arg, void *data, int flags)
249
static int find_channel_by_group(void *obj, void *arg, void *data, int flags)
244
{
250
{
245
	struct ast_channel *chan = obj;
251
	struct ast_channel *chan = obj;
246
	struct ast_channel *c = data;
252
	struct ast_channel *c = data;
247

    
   
253

   
248
	ast_channel_lock(chan);
254
	ast_channel_lock(chan);
249
	int i = (c != chan) &&
255
	int i = (c != chan) &&
250
		(chan->pickupgroup & c->callgroup) &&
256
		(chan->pickupgroup & c->callgroup) &&
251
		!c->masq && can_pickup(chan);
257
		can_pickup(chan);
252

    
   
258

   
253
	ast_channel_unlock(chan);
259
	ast_channel_unlock(chan);
254
	return i ? CMP_MATCH | CMP_STOP : 0;
260
	return i ? CMP_MATCH | CMP_STOP : 0;
255
}
261
}
256

    
   
262

   
257
static int pickup_by_group(struct ast_channel *chan)
263
static int pickup_by_group(struct ast_channel *chan)
258
{
264
{
259
	struct ast_channel *target;
265
	struct ast_channel *target;
260
	int res = -1;
266
	int res = -1;
261
	if ((target = ast_channel_callback(find_channel_by_group, NULL, chan, 0))) {
267

   

    
   
268
	if (!(target = ast_channel_callback(find_channel_by_group, NULL, chan, 0))) {

    
   
269
		return res;

    
   
270
	}

    
   
271

   

    
   
272
	ast_log(LOG_NOTICE, "%s, pickup attempt by %s\n", target->name, chan->name);
262
		ast_channel_lock(target);
273
	ast_channel_lock(target);

    
   
274
	if (can_pickup(target)) {
263
		res = ast_do_pickup(chan, target);
275
		res = ast_do_pickup(chan, target);

    
   
276
	} else {

    
   
277
		ast_log(LOG_WARNING, "target has gone, or not ringing anymore for %s\n", chan->name);

    
   
278
	}
264
		ast_channel_unlock(target);
279
	ast_channel_unlock(target);
265
		target = ast_channel_unref(target);
280
	target = ast_channel_unref(target);
266
	}
281

   
267
	return res;
282
	return res;
268
}
283
}
269

    
   
284

   
270
/* application entry point for Pickup() */
285
/* application entry point for Pickup() */
271
static int pickup_exec(struct ast_channel *chan, const char *data)
286
static int pickup_exec(struct ast_channel *chan, const char *data)
[+20] [20] 4 lines
[+20] static int pickup_by_group(struct ast_channel *chan)
276

    
   
291

   
277
	if (ast_strlen_zero(data)) {
292
	if (ast_strlen_zero(data)) {
278
		res = pickup_by_group(chan);
293
		res = pickup_by_group(chan);
279
		return res;
294
		return res;
280
	}
295
	}
281
	
296

   
282
	/* Parse extension (and context if there) */
297
	/* Parse extension (and context if there) */
283
	while (!ast_strlen_zero(tmp) && (exten = strsep(&tmp, "&"))) {
298
	while (!ast_strlen_zero(tmp) && (exten = strsep(&tmp, "&"))) {
284
		if ((context = strchr(exten, '@')))
299
		if ((context = strchr(exten, '@')))
285
			*context++ = '\0';
300
			*context++ = '\0';
286
		if (!ast_strlen_zero(context) && !strcasecmp(context, PICKUPMARK)) {
301
		if (!ast_strlen_zero(context) && !strcasecmp(context, PICKUPMARK)) {
[+20] [20] 32 lines
[+20] [+] static int pickup_by_part(struct ast_channel *chan, const char *part)
319
	struct ast_channel *target;
334
	struct ast_channel *target;
320
	int res = -1;
335
	int res = -1;
321

    
   
336

   
322
	if ((target = ast_channel_callback(find_by_part, NULL, (char *) part, 0))) {
337
	if ((target = ast_channel_callback(find_by_part, NULL, (char *) part, 0))) {
323
		ast_channel_lock(target);
338
		ast_channel_lock(target);

    
   
339
		if (can_pickup(target)) {
324
		res = ast_do_pickup(chan, target);
340
			res = ast_do_pickup(chan, target);

    
   
341
		} else {

    
   
342
			ast_log(LOG_WARNING, "target has gone, or not ringing anymore for %s\n", chan->name);

    
   
343
		}
325
		ast_channel_unlock(target);
344
		ast_channel_unlock(target);
326
		target = ast_channel_unref(target);
345
		target = ast_channel_unref(target);
327
	}
346
	}
328

    
   
347

   
329
	return res;
348
	return res;
[+20] [20] 64 lines
trunk/channels/chan_sip.c
Diff Revision 5 Diff Revision 8
 
trunk/include/asterisk/features.h
Diff Revision 5 Diff Revision 8
 
trunk/main/features.c
Diff Revision 5 Diff Revision 8
 
trunk/res/res_features.exports.in
Diff Revision 5 Diff Revision 8
 
  1. trunk/apps/app_directed_pickup.c: Loading...
  2. trunk/channels/chan_sip.c: Loading...
  3. trunk/include/asterisk/features.h: Loading...
  4. trunk/main/features.c: Loading...
  5. trunk/res/res_features.exports.in: 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.