[Asterisk-code-review] res_pjsip_refer: Always serialize calls to refer_progress_notify (asterisk[16])

[George Joseph]

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

Change subject: res_pjsip_refer: Always serialize calls to refer_progress_notify
......................................................................


Patch Set 1:

(3 comments)

https://gerrit.asterisk.org/c/asterisk/+/15400/1/include/asterisk/res_pjsip.h 
File include/asterisk/res_pjsip.h:

https://gerrit.asterisk.org/c/asterisk/+/15400/1/include/asterisk/res_pjsip.h@1693 
PS1, Line 1693: int ast_sip_push_task_serializer(struct ast_taskprocessor *serializer, int (*sip_task)(void *), void *task_data);
> The return documentation is incorrect.  You cannot return the sip_task() value as there is no way for you to return it if it actually got pushed.  You can only return success or failure to push.

cut and paste error.  will fix.


https://gerrit.asterisk.org/c/asterisk/+/15400/1/res/res_pjsip.c 
File res/res_pjsip.c:

https://gerrit.asterisk.org/c/asterisk/+/15400/1/res/res_pjsip.c@4762 
PS1, Line 4762: int ast_sip_push_task_serializer(struct ast_taskprocessor *serializer, int (*sip_task)(void *), void *task_data)
> I question the need for this function as we already have ast_sip_push_task().

ast_sip_push_task doesn't execute the callback in-line if it't being called from the serializer already.


https://gerrit.asterisk.org/c/asterisk/+/15400/1/res/res_pjsip_refer.c 
File res/res_pjsip_refer.c:

https://gerrit.asterisk.org/c/asterisk/+/15400/1/res/res_pjsip_refer.c@566 
PS1, Line 566: 			if (ast_sip_push_task_serializer(attended->progress->serializer, refer_progress_notify, notification)) {
> Why not use ast_sip_push_task()?  The same for the other places where you are using the new ast_sip_push_task_serializer().

Because some of these places might already be on the progress serializer and i didn't want to alter the timing by tossing the notifies back on the serializer queue.  It _does_ make a difference and I didn't want to introduce possible issues by altering the timing.



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

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: Idcf1934c4e873f2c82e2d106f8d9f040caf9fa1e
Gerrit-Change-Number: 15400
Gerrit-PatchSet: 1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Tue, 09 Feb 2021 20:52:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Richard Mudgett <rmudgett at digium.com>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210209/3d59ec94/attachment.html>