[Asterisk-code-review] res pjsip: Add serialized scheduler (res pjsip/pjsip schedu... (asterisk[13])

[George Joseph]

George Joseph has posted comments on this change.

Change subject: res_pjsip:  Add serialized scheduler (res_pjsip/pjsip_scheduler.c)
......................................................................


Patch Set 4:

(4 comments)

https://gerrit.asterisk.org/#/c/2443/4/include/asterisk/res_pjsip.h
File include/asterisk/res_pjsip.h:

Line 1364: 	AST_SIP_SCHED_TASK_FIXED = (0 << 0),
> I don't understand why these 0 bit-shifted flags exist. It looks like they 
Following the pattern in astobj2. :)
Makes things a little clearer since you can attach documentation to them.


https://gerrit.asterisk.org/#/c/2443/4/res/res_pjsip/pjsip_scheduler.c
File res/res_pjsip/pjsip_scheduler.c:

PS4, Line 41: 	/*! ast_sip_sched task id */
            : 	int task_id;
> Given how this gets stringified on initialization, you may want to make thi
Done


PS4, Line 160: 	res = ast_sched_del(scheduler_context, schtd->current_scheduler_id);
             : 	ao2_unlock_and_unref(schtd);
             : 	ao2_unlink(tasks, schtd);
> Things can get a bit bumpy here. ast_sched_del() can potentially deadlock. 
Done


Line 252: 			since_when = schtd->is_running ? ast_tvnow() : schtd->last_end;
> I think the is_running logic here is skewed. If the task is currently runni
Done


-- 
To view, visit https://gerrit.asterisk.org/2443
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7af6ad2b2d896ea68e478aa1ae201d6dd016ba1c
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes