> On Aug. 6, 2014, 4 p.m., rmudgett wrote: > > /team/group/rls/res/res_pjsip_pubsub.c, line 1137 > > <https://reviewboard.asterisk.org/r/3723/diff/6/?file=66266#file66266line1137> > > > > It might be better to remove_subscription() first before destroying the > > sub_tree contents. > > > > remove_subscription() also does a module unref which would need to be > > moved to unlink the subscription from the list first. > > Mark Michelson wrote: > I'm getting a parse error on your second sentence, specificallly "which > would need to be moved to unlink the subscription from the list first." The > module unref is already happening after the subscription is unlinked from the > list.
Changing when the sub_tree is unlinked from the subscriptions list to the beginning should not also unref the module. The module would need to be unreffed at the end of subscription_setup_dialog() after the sub_tree resources are destroyed. Is the subscription list containing the sub_tree accessed by other threads? It would seem so since the list is protected by a rwlock. It makes sense to remove the sub_tree from the subscription list (and thus access by other threads) before destroying the object and its resources. Once that is done then the object's module should be unreffed. - rmudgett ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3723/#review13033 ----------------------------------------------------------- On Aug. 6, 2014, 5 p.m., Mark Michelson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3723/ > ----------------------------------------------------------- > > (Updated Aug. 6, 2014, 5 p.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-23869 > https://issues.asterisk.org/jira/browse/ASTERISK-23869 > > > Repository: Asterisk > > > Description > ------- > > This adds NOTIFY support for resource list subscriptions. The way this works > is as follows: > > When an initial SUBSCRIBE arrives and the subscription tree is built, all > leaf nodes are called into in order to generate their initial NOTIFY bodies > and store these on their respective subscription nodes. > > Sending a NOTIFY requires traversing the tree. List subscriptions will > generate a multipart/related body with an RLMI part and parts corresponding > to the leaves of the list (at least they will eventually. ASTERISK-23867 > involves doing this part). Single-resource subscriptions read the stored body > on the subscription and uses that to populate the NOTIFY body. > > Leaf nodes in the subscription tree, when they have a state change occur, > call ast_sip_subscription_notify(), as they previously did. > ast_sip_subscription_notify() creates the NOTIFY body for the subscription > and stores that on the subscription in the body_text ast_str field. The > subscription tree is then told to send a NOTIFY if no batching is enabled or > to start a batched NOTIFY if batching is enabled. > > When a resource resubscribes or terminates their subscription, a NOTIFY is > now automatically generated by the pubsub core instead of calling into > subscription handlers. The NOTIFY is built the same way as previously, using > stored NOTIFY bodies on the subscription. This NOTIFY can also cause batched > notification, when the timer fires, not to actually send their batch since it > would be redundant. > > You'll notice the code has been refactored slightly, and a new struct, > sip_subscription_tree, has invaded res_pjsip_pubsub. This is because, as I > was separating the "real" and "virtual" parts of ast_sip_subscriptions out, I > realized that I essentially had two distinct structures. Thus, I separated > the real/meta/base elements of a subscription into the sip_subscription_tree, > and the resource-specific parts into the ast_sip_subscription struct. > sip_subscription_tree is used more heavily in the pubsub core now, whereas > ast_sip_subscription acts as a handle for subscription handlers to use plus a > repository for resource-specific data. > > > Diffs > ----- > > /team/group/rls/res/res_pjsip_pubsub.c 420264 > /team/group/rls/res/res_pjsip_mwi.c 420264 > /team/group/rls/res/res_pjsip_exten_state.c 420264 > /team/group/rls/include/asterisk/res_pjsip_pubsub.h 420264 > > Diff: https://reviewboard.asterisk.org/r/3723/diff/ > > > Testing > ------- > > With this set of changes, I'm still not able to perform RLS-specific tests > since there is still no method of generating multipart/related or RLMI > bodies. However, with these changes, I did run the gamut of subscription > tests in the testsuite and they all pass. This at least means that there are > no detectable regressions at this point. > > > Thanks, > > Mark Michelson > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
