Summary:	ASTERISK-03364: [patch] Return uniqueid with astman event OriginateSuccess
Reporter:	magg (magg)	Labels:
Date Opened:	2005-01-27 03:52:29.000-0600	Date Closed:	2008-01-15 15:24:03.000-0600
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) 20050127__originate_success.diff.txt ( 1) 20050201__originate_success.diff.txt
Description:	Hi, 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? :) NB 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) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=4948