[Asterisk-code-review] res pjsip outbound publish: Add multi-user support per confi... (asterisk[master])
Kevin Harwell
asteriskteam at digium.com
Wed Apr 27 16:22:56 CDT 2016
Kevin Harwell has posted comments on this change.
Change subject: res_pjsip_outbound_publish: Add multi-user support per configuration
......................................................................
Patch Set 1:
(4 comments)
https://gerrit.asterisk.org/#/c/2709/1/include/asterisk/res_pjsip_outbound_publish.h
File include/asterisk/res_pjsip_outbound_publish.h:
PS1, Line 174: int ast_sip_publish_user_send(const char *user, const struct ast_sip_body *body);
:
: /*!
: * \brief Remove the given user and stop outbound publishing for it
: *
: * \param user The user to remove
: */
: void ast_sip_publish_user_remove(const char *user);
> This has to still take in a client. Each client has a set of users. Users s
What do you mean by "Users shouldn't be global"? For this function I could see how just passing in client (I'm guessing obtained from the datastore) alone could then be used to look up all associated states. Is that what you mean?
https://gerrit.asterisk.org/#/c/2709/1/res/res_pjsip_outbound_publish.c
File res/res_pjsip_outbound_publish.c:
Line 200: static struct ao2_container *multi_publishes;
> Why did this need to be a global container instead of a container on the pu
There is only one publish [configuration] type per publish client. However now for a given publish type there may be 'n' publish clients (it was 1 to 1).
Publish clients are created when the state gets created (so still a 1 to 1 state/client relation). For a multi-user publish type the creation of the state is postponed until it is dynamically created in the client_user_send code. Because of this we need a mechanism to store the multi-user publish configuration type objects until such time, thus the global container.
Line 264: static AO2_GLOBAL_OBJ_STATIC(current_states);
> I don't think it makes sense anymore to have a global obj container that's
It may not need to be anymore? However there are a few 'read' cases where it would be bad if the container was "swapped" out from underneath during one of these 'reads'.
Line 824: ao2_callback_data(multi_publishes, OBJ_NODATA,
> Doesn't this introduce a race condition where if another thread does this a
I thought the ao2_callback code would mutex lock the multi_publishes container? A second thread would be locked out until the first completed the callback/iteration.
--
To view, visit https://gerrit.asterisk.org/2709
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib88dde024cc83c916424645d4f5bb84a0fa936cc
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-HasComments: Yes
More information about the asterisk-code-review
mailing list