Bug in gluon/scheduler.py

47 views
Skip to first unread message

Tom Clerckx

unread,
Aug 8, 2022, 6:50:26 AMAug 8
to web2py-users
Version:
Version 2.22.5-stable+timestamp.2022.06.04.18.13.51

There is a problem with the calculation of next_run_time in gluon/scheduler.py at line 1024

It calculates:
steps = secondspassed // task.period + 1

However, there is no check done for task.period being 0

This can cause the following scheduler error:
ZeroDivisionError: float divmod()

I think it would be better to initialize next_run_time and change the last else condition to "elif task.period".

Dave S

unread,
Aug 19, 2022, 6:49:05 AMAug 19
to web2py-users
How did you get task.period == 0?

I'm wondering if this is a valid use-case; the scheduler has quite a test suite.

/dps
 

Tom Clerckx

unread,
Aug 20, 2022, 2:39:55 PMAug 20
to web2py-users
Our tasks are added using scheduler.queue_task(...) function.

By default, we set the period to 0 
Later on we added to the defaults : prevent_drift = True

It's this last change that triggered the observed error.

Dave S

unread,
Aug 21, 2022, 7:03:12 AMAug 21
to web2py-users
On Saturday, August 20, 2022 at 11:39:55 AM UTC-7 Tom Clerckx wrote:
Our tasks are added using scheduler.queue_task(...) function.

By default, we set the period to 0 

Why?  If you don't know the period, why are you queuing it?   Or are you using the "cron" type of task scheduling? 

 
Later on we added to the defaults : prevent_drift = True

It's this last change that triggered the observed error.


Eveything I know about the scheduler has been shown to me by Stefan (Niphlod), who wrote the subsystem and the tests.  I suspect I would have to spend quite a bit of time to come up with an analysis of what is really wrong, but I'm really suspicious of queuing a period of 0.

/dps

Tom Clerckx

unread,
Aug 21, 2022, 2:10:33 PMAug 21
to web2py-users
We usually set the period to 0 for tasks that run only once.
Hence, the period should not matter at all.

This works fine, unless you also set the prevent_drift to True -- this causes the division by zero.

I agree that it is a bit silly to set prevent_drift=True for a task that runs only once.
I also believe it makes sense to make the code more robust. 

I think it is quite easy to fix this in the code as I mentioned in the initial post.
See the diff in attached screenshot:
The 'next_run_time' doesn't matter when it's a one-time task, so it can be initialized outside of the condition.
And recalculating it using task.period only makes sense if task.period is bigger than 0.

Tom.

2022-08-21-scheduler-diff.png
Reply all
Reply to author
Forward
0 new messages