Summary:ASTERISK-03364: [patch] Return uniqueid with astman event OriginateSuccess
Reporter:magg (magg)Labels:
Date Opened:2005-01-27 03:52:29.000-0600Date Closed:2008-01-15 15:24:03.000-0600
Versions:Frequency of
Environment:Attachments:( 0) 20050127__originate_success.diff.txt
( 1) 20050201__originate_success.diff.txt

Is it possible to return the uniqueid of the new call with in the OriginateSuccess event?
Comments:By: Tilghman Lesher (tilghman) 2005-01-27 12:28:41.000-0600

Try this patch.  Disclaimer on file.

By: magg (magg) 2005-01-31 03:26:17.000-0600

This seems to work just fine!
I had to merge a little by hand, because of some other changes to manager.c, but besides that, no problem!

(I've just tested a few calls yet)

edited on: 01-31-05 03:26

By: Mark Spencer (markster) 2005-01-31 22:42:14.000-0600

Hrm, this patch could potentially introduce a race, right?  Would it perhaps be better to simply pass back the uniqueid?

By: Tilghman Lesher (tilghman) 2005-01-31 23:21:56.000-0600

I doubt it's worth it.  You're going to have that same race either way, unless you lock the channel before returning from ast_request_and_dial().

By: Mark Spencer (markster) 2005-01-31 23:26:48.000-0600

To clarify, my suggestion was to pass a buffer (and length) into which the unique ID would be copied immediately after channel allocation.

By: Mark Spencer (markster) 2005-01-31 23:27:28.000-0600

The "locked channel" idea also would potentially be a good one (but only lock if the channel arg is passed).

By: Tilghman Lesher (tilghman) 2005-02-01 00:01:53.000-0600

Do we really want to go down the road of locking it?  The only safe place to enter that lock is before we call ast_pbx_run() on the newly created channel.  Any place after that is a potential race condition between the channel exiting and the lock attempted on a deallocated structure.  Do you see what I mean?  There's already a race condition inherent in the fact that we're setting channel structure members in ast_request_and_dial() without having the underlying channel locked.

I just don't see that locking the channel in ast_request_and_dial() before we return to manager.c is really going to protect you from the race condition that already exists.  If we really want to handle the race condition, that's fine, but it needs to happen in each channel's new before it calls ast_pbx_run().

By: Mark Spencer (markster) 2005-02-01 00:08:17.000-0600

Until the thread / pbx is spawned, your channel is safe because you're the only thread having it.  If we lock it before launching the thread, then basically the thread will block on the first call trying to lock.   I would recommend either the lock-the-channel method *or* just returning the uniqueid.

By: Tilghman Lesher (tilghman) 2005-02-01 00:38:22.000-0600

Since ast_pbx_run() creates the thread, though, that's the point at which we have uncertainty about whether the channel exits before we're done setting it up.  That's why the lock must happen before we call ast_pbx_run() in the various channels.

For example, take chan_zap as a sample case:
a) manager.c calls ast_request_and_dial
b) ast_request_and_dial calls ast_request
c) ast_request calls zt_request
d) zt_request restarts the monitor (triggering handle_init_event, which starts the channel thread) <-- Uncertainty enters here.
e) ast_request_and_dial sets more channel structure elements
f) manager.c references chan->uniqueid

In other words, the entry of manager.c referencing chan->uniqueid does not, in itself, create any new race conditions; at worst, it merely extends a preexisting race condition (i.e. everything in (e)).

By: Tilghman Lesher (tilghman) 2005-02-01 00:41:34.000-0600

To clarify, it isn't just ast_pbx_run() that is the problem:  at any point after we either restart the monitor thread or invoke thread creation, we have uncertainty, where a lock would certainly help; but this bugset patch is not the cause of the race.

By: Mark Spencer (markster) 2005-02-01 00:44:34.000-0600

I think you're confusing an inbound and outbound call flow.  zt_request does not start the PBX unless it's an *inbound* call, not in the case of an outbound one.

By: Tilghman Lesher (tilghman) 2005-02-01 00:48:55.000-0600

Moved this discussion to bugset ASTERISK-3405, since it's not related to this patch.

By: nick (nick) 2005-02-01 19:44:43.000-0600

So its the lock-the-channel road then? :)


By: Mark Spencer (markster) 2005-02-01 20:53:05.000-0600

Added to CVS with changes, thanks!

By: magg (magg) 2005-02-02 02:50:47.000-0600

seems like I got Corydon76's karma points ;)

By: Mark Spencer (markster) 2005-02-02 10:54:35.000-0600

you did, but i also gave them to him.  i just didn't want to give you bad karma again, so just consider yourself lucky :)

By: Russell Bryant (russell) 2005-02-06 21:41:10.000-0600

not in 1.0

By: Digium Subversion (svnbot) 2008-01-15 15:24:03.000-0600

Repository: asterisk
Revision: 4948

U   trunk/include/asterisk/pbx.h
U   trunk/manager.c
U   trunk/pbx/pbx_spool.c
U   trunk/pbx.c

r4948 | markster | 2008-01-15 15:24:02 -0600 (Tue, 15 Jan 2008) | 2 lines

Include uniqueid in response for ManagerOriginate stuff (bug ASTERISK-3364)