[Home]

Summary:ASTERISK-03224: [PATCH] simplify ASTOBJ lifetime mgmt API
Reporter:Kevin P. Fleming (kpfleming)Labels:
Date Opened:2005-01-08 18:30:40.000-0600Date Closed:2008-01-15 15:21:03.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) astobj_simple_destroy_rev2.diff.txt
Description:The attached patch simplfies the management of ASTOBJ object lifetimes. It removes the "DELME" flag and the ASTOBJ_DESTROY macro, since they are not really necessary.

Instead, objects are directly destroyed when ASTOBJ_UNREF is called and their refcount drops to zero.

Along the way in making this patch I had to follow through the chan_sip code to ensure that I wasn't causing any breakage; I found the code in realtime_peer and realtime_user so hard to follow (and inefficient) that I rewrote it... sorry for bundling that with this patch, but it was part of the same work :-)

****** ADDITIONAL INFORMATION ******

Disclaimer is on file.
Comments:By: Mark Spencer (markster) 2005-01-09 03:14:21.000-0600

I don't think I buy into the realtime changes being more efficient...  Now we traverse the list of variables not just twice, but three times.  The liklihood of type= being wrong is generally pretty rare, but in any case if you want to change it, make it take only one pass, by fixing build_peer and build_user so that when building realtime, those parameters are checked there.

By: Kevin P. Fleming (kpfleming) 2005-01-09 07:28:00.000-0600

I was definitely making an assumption about the realtime aspects, as I couldn't confirm how likely it was that the type would be "wrong". It's definitely more efficent to check the list over once, though, than to go through all the work that build_peer does and then throw it away :-)

I'll see if there's a way to implement your suggestion... stay tuned.

By: Kevin P. Fleming (kpfleming) 2005-01-09 08:21:23.000-0600

New version, removes extra variable processing loop from realtime_peer. build_peer and build_user now take an additional argument to signify building of a "realtime" entry (build_user does not thing with it, but I added it for consistency's sake).

Some of the variables processed in the realtime_peer loop were already handled by build_peer, so there was some duplication removed as well (specifically "host=dynamic" and "port", although "port" handling is slightly different for realtime and that behavior was preserved).

By: Mark Spencer (markster) 2005-01-09 23:44:39.000-0600

Added to CVS with a couple of mods:

1) if (realtime) { /* check for things */ } else { /* check for things */ } would seem to preclude regular options from working with realtime, so i changed that.

2) the "realtime" and "temponly" options had exactly the same meaning.  I stuck with "realtime" to make it clear.

mark

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

Repository: asterisk
Revision: 4747

U   trunk/channels/chan_sip.c
U   trunk/include/asterisk/astobj.h

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

Improve object destruction (bug ASTERISK-3224)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=4747