[Asterisk-code-review] bridges: Remove reliance on stasis caching (asterisk[master])

[George Joseph]

George Joseph has posted comments on this change. ( https://gerrit.asterisk.org/10227 )

Change subject: bridges:  Remove reliance on stasis caching
......................................................................


Patch Set 2:

(3 comments)

https://gerrit.asterisk.org/#/c/10227/2/include/asterisk/stasis_bridges.h
File include/asterisk/stasis_bridges.h:

https://gerrit.asterisk.org/#/c/10227/2/include/asterisk/stasis_bridges.h@73
PS2, Line 73: * \since 13.24
            :  * \since 15.6
            :  * \since 16.1
> Still here :-)

Bah.


https://gerrit.asterisk.org/#/c/10227/2/main/stasis_bridges.c
File main/stasis_bridges.c:

https://gerrit.asterisk.org/#/c/10227/2/main/stasis_bridges.c@296
PS2, Line 296: 	update->old_snapshot = old;
             : 	update->new_snapshot = new;
> Since you are calling ao2_cleanup on these in the destructor these
 > should probably get a ref bump here at assignment.

We're basically transferring the refcounts to the new update object.  See below.


https://gerrit.asterisk.org/#/c/10227/2/main/stasis_bridges.c@383
PS2, Line 383: 	ao2_ref(update, -1);
             : 	if (!msg) {
             : 		return;
             : 	}
             : 
             : 	/* The link of the new automatically replaces the old */
             : 	ao2_link(bridge_cache, new_snapshot);
> If I am tracing the code right, then new_snapshot currently has a
 > single ref (update_create doesn't currently bump it). As does
 > 'update'. Update gets unref'd, the destructor is called and
 > new_snapshot gets cleaned up in update's destructor. Then it gets
 > linked, but at this point it seems like it has been potentially
 > destroyed?

No because stasis_message_create bumps update if the message is successfully created.  Therefore the unref will only call the destructor if the message create failed and didn't bump the refcount.  In that case, we just return anyway.



-- 
To view, visit https://gerrit.asterisk.org/10227
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7049b80efa88676ce5c4666f818fa18ad1985369
Gerrit-Change-Number: 10227
Gerrit-PatchSet: 2
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2 (1000185)
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Comment-Date: Wed, 26 Sep 2018 15:59:16 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180926/766e86b9/attachment-0001.html>