[Home]

Summary:ASTERISK-22081: Type cast causes incorrect schedule interval
Reporter:Roman S. (rolesok)Labels:
Date Opened:2013-07-16 02:09:32Date Closed:
Priority:TrivialRegression?
Status:Open/NewComponents:General
Versions:1.8.23.0 10.12.2 11.5.0 13.18.4 Frequency of
Occurrence
Related
Issues:
Environment:linux 32 and 64Attachments:( 0) sched-22081.diff
Description:If there is a single event with {{when}} in the distant future then function {{ast_sched_wait}} returns zero value (no wait - run it now). This causes high CPU utilization.

This happens when {{ast_tvdiff_ms}} returns value of type {{int64_t}} which is greater then INT_MAX ({{ms}} of type {{int}}) - negative {{int}} value occurs).

{noformat}
int ast_sched_wait(struct ast_sched_context *con)
{
       int ms;
...
               ms = ast_tvdiff_ms(s->when, ast_tvnow());
{noformat}

{noformat}
int64_t ast_tvdiff_ms(struct timeval end, struct timeval start),
{noformat}

For 32bit OS it's 24 days schedule interval - big enough, but still possible.

I suppose that type change and additional check is required:
{noformat}
       int64_t ms;
...
               if (ms > INT_MAX) {
                       ms = -1;
               else if (ms < 0) {
...
{noformat}
Comments:By: Matt Jordan (mjordan) 2013-07-19 11:36:34.292-0500

In general, we can't accept inline patches or code suggestions. That may sound crazy for one-liners, but we want to be very careful that any code submission is properly attributed and licensed.

Would you mind attaching the suggestion in unified diff format to this issue?

Thanks!
By: Rusty Newton (rnewton) 2013-08-06 17:58:11.034-0500

Roman would you mind following through with the [Patch and Code Submission guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines#AsteriskIssueGuidelines-PatchandCodesubmission]?

Primarily, as Matt mentioned above, signing the contributor's agreement and uploading the patch as a contribution?
By: Rusty Newton (rnewton) 2013-08-21 12:58:44.063-0500

Considering the trivial nature of this issue and since you have signed the submission agreement, provided this report as a suggestion and not as a patch we'll go ahead and acknowledge this for someone to fix.
By: Roman S. (rolesok) 2013-08-23 00:35:54.487-0500

Fix as suggested earlier.
P.S. Sorry for delay.
By: Kinsey Moore (kmoore) 2013-09-18 15:05:32.246-0500

The fix provided here, while better-behaving in your situation, may cause problems with other users of ast_sched_wait since it reports that no events are scheduled to occur if the next event is sufficiently far off. This is contrary to documentation in sched.h.

The correct fix is a bit more far reaching and involves correcting handling and return types where necessary for functions using ast_sched_wait and ast_tvdiff_ms.
By: Kinsey Moore (kmoore) 2013-09-24 08:57:03.926-0500

Further details on the more invasive fix:
* Change the return type of ast_sched_wait to int64_t and correct its internal handling of ast_tvdiff_ms.
* Update all callers of ast_sched_wait with the new return type and ensure they handle it properly.

If someone wanted to go further, there are other callers of ast_tvdiff_* that need to be fixed as well. Callers of ast_tvdiff_sec appear to be written correctly.