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

Kevin Harwell asteriskteam at digium.com
Wed Sep 26 10:21:57 CDT 2018


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

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


Patch Set 2: Code-Review-1

(5 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 :-)


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.


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?


https://gerrit.asterisk.org/#/c/10227/2/main/stasis_bridges.c@1374
PS2, Line 1374:  * \since 12.0.0
Delete this line


https://gerrit.asterisk.org/#/c/10227/2/main/stasis_bridges.c@1412
PS2, Line 1412:  * \since 12.0.0
Remove this line too.



-- 
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:21:57 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180926/fb986d40/attachment.html>


More information about the asterisk-code-review mailing list