classification
Title: Use time.steady() to implement timeout
Type: behavior Stage:
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution:
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: aljungberg, anacrolix, haypo, neologix, pitrou, python-dev, rhettinger, tasslehoff
Priority: normal Keywords: patch

Created on 2012-03-07 19:38 by tasslehoff, last changed 2014-02-25 22:26 by haypo. This issue is now closed.

Files
File name Uploaded Description Edit
queue_monotonic.patch haypo, 2012-03-14 00:53 review
threading_monotonic.patch haypo, 2012-03-14 00:56 review
Messages (19)
msg155106 - (view) Author: Tasslehoff Kjappfot (tasslehoff) Date: 2012-03-07 19:38
Queue.get([block[, timeout]]) uses time.time() for calculations of the timeout. If someone changes the system time this breaks. If a timeout of 1 second is set and someone (on a unix system) uses date to set the time 1 hour back, Queue.get will use 1 hour and 1 second to time out.

I'm guessing the fix is just to use another library function to measure time, but I don't know if one exists that works on all platforms.
msg155129 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-03-07 22:40
Well, in 3.3 we could use clock_gettime(CLOCK_MONOTONIC) where available.
That said, this is not specific to Queue.get() and will probably happen with many similar functions taking a timeout parameter.
msg155139 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2012-03-08 00:38
> Well, in 3.3 we could use clock_gettime(CLOCK_MONOTONIC) where available.

It's better to use time.monotonic().

> That said, this is not specific to Queue.get() and will probably happen with many similar functions taking a timeout parameter.

Yep, it may be used for lock.acquire(timeout=timeout) for example. It 
would help to have a portable monotonic clock API in C.
msg155701 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2012-03-14 00:53
queue_monotonic.patch: simple patch using time.monotonic(), with a fallback to time.time() if monotonic failed or is not available.
msg155703 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2012-03-14 00:56
threading_monotonic.patch: similar patch for the threading module.

Drawback of these patchs: they now call time.monotonic() when the module is loaded (another useless syscall?).
msg155770 - (view) Author: Charles-Fran├žois Natali (neologix) * (Python committer) Date: 2012-03-14 17:02
Random thoughts:
- as noted by Antoine, it probably affects a lot of code throughout
the standard library
- sem_timedwait() (used by lock.acquire() on POSIX) is affected (the
implementation using a condition variable could use
pthread_condattr_setclock(), see issue #12822)
- there's a side effect to that change: on Linux, when the system is
suspended, CLOCK_MONOTONIC stops: so on resume, you'd have to wait for
the complete timeout to expire, whereas with time.time() you'll break
out early

That being said, it's probably a good idea.

As for the patches, I don't really like the idea of having to use this
idiom everywhere:

try:
    from time import monotonic as _time
    _time()
except (ImportError, OSError):
    from time import _time
msg155828 - (view) Author: Roundup Robot (python-dev) Date: 2012-03-15 00:22
New changeset d97ae725814c by Victor Stinner in branch 'default':
Issue #14222: Use the new time.steady() function instead of time.time() for
http://hg.python.org/cpython/rev/d97ae725814c
msg155833 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2012-03-15 01:07
Yo, I had marked this as something I was going to review once the python-dev discussion to complete.  FWIW, I'm the maintainer of the Queue module.
msg156330 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2012-03-19 12:13
> Yo, I had marked this as something I was going to review once
> the python-dev discussion to complete.  FWIW, I'm the maintainer
> of the Queue module.

And so, do you agree with the change?
msg157521 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2012-04-04 23:54
Other modules that should use a monotonic clock instead of the system time:

 - timeit: default_timer = time.time (but time.clock is still the best clock on Windows for this mmodule)
 - subprocess for Popen.communicate() timeoout
 - sched
 - socket, ssl: timeout. need a C function which uses a monotonic clock if available, or fallback to the system clock
 - trace

See also issues #12338 and #14309.
msg157522 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2012-04-05 00:55
I don't think *trace* and *timeit* should be changed.
msg157532 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2012-04-05 02:58
Why do you think monotonic time is needed for the Queue module?  If time.time() goes backwards for some reason, the only consequence is that the timeouts take longer to cross the timeout boundard.  On the other hand, it monotonic is used, then time won't go backwards but it won't go forwards either, so the consequence is the same.

AFAICT, this patch doesn't fix any bug and doesn't make the module better in any observable way.
msg157564 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2012-04-05 11:29
> Why do you think monotonic time is needed for the Queue module?

The duration of a timeout of N seconds should be N seconds even if the system clock is updated (e.g. a daylight saving time (DST) change).

This feature is checked by an unit test in the patch attached to the isuse #14428 (implementation of the PEP 418):

+    def test_monotonic_settime(self):
+        t1 = time.monotonic()
+        realtime = time.clock_gettime(time.CLOCK_REALTIME)
+        # jump backward with an offset of 1 hour
+        time.clock_settime(time.CLOCK_REALTIME, realtime - 3600)
+        t2 = time.monotonic()
+        time.clock_settime(time.CLOCK_REALTIME, realtime)
+        # monotonic must not be affected by system clock updates
+        self.assertGreaterEqual(t2, t1)

You can imagine a similar patch for Queue timeout.
msg157565 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2012-04-05 11:29
> You can imagine a similar patch for Queue timeout.

Oops: You can imagine a similar *test* for Queue timeout.
msg157766 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2012-04-08 00:48
[Victor]
> The duration of a timeout of N seconds should be N seconds even 
> if the system clock is updated (e.g. a daylight saving time 
> (DST) change).

IIRC, time.time() is not a local time and it is unaffected by a DST change.
msg157767 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2012-04-08 01:06
I'm closing this one.  The "not subject to adjustment" property is more desirable than not.  The "undefined reference point" property isn't harmful in this context (the start time isn't exposed).  

I'll follow the python-dev thread and re-open this if it becomes clear that there are any issues for steady() being used for timeout logic.
msg157782 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-08 10:00
> The "not subject to adjustment" property is more desirable than not.
> The "undefined reference point" property isn't harmful in this context 
> (the start time isn't exposed).

So you're closing the issue while you're in agreement with it?
msg212225 - (view) Author: Alexander Ljungberg (aljungberg) Date: 2014-02-25 22:05
This still appears to be an issue in Python 2.7. Queue.get routinely hangs for a very long time on the Raspberry Pi as it doesn't have a clock battery and often ends up significantly adjusting its system time soon after startup.
msg212227 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2014-02-25 22:26
You should upgrade to python 3.3! The pep 418 mentions different available
modules for python 2. My new trollius project has for example an
implementation in asyncio.time_monotonic.
History
Date User Action Args
2014-02-25 22:26:22hayposetmessages: + msg212227
2014-02-25 22:05:40aljungbergsetnosy: + aljungberg
messages: + msg212225
2012-04-08 10:00:36pitrousetmessages: + msg157782
2012-04-08 01:06:21rhettingersetstatus: open -> closed

messages: + msg157767
2012-04-08 00:48:57rhettingersetmessages: + msg157766
2012-04-05 11:29:42hayposetmessages: + msg157565
2012-04-05 11:29:23hayposetmessages: + msg157564
2012-04-05 02:58:46rhettingersetmessages: + msg157532
2012-04-05 00:55:26rhettingersetmessages: + msg157522
2012-04-04 23:54:34hayposetmessages: + msg157521
2012-03-19 12:50:26hayposettitle: Using time.time() in Queue.get breaks when system time is changed -> Use time.steady() to implement timeout
2012-03-19 12:13:21hayposetmessages: + msg156330
2012-03-15 01:07:27rhettingersetmessages: + msg155833
2012-03-15 00:22:04python-devsetnosy: + python-dev
messages: + msg155828
2012-03-14 21:06:47rhettingersetassignee: rhettinger

nosy: + rhettinger
2012-03-14 17:02:42neologixsetmessages: + msg155770
2012-03-14 16:26:22anacrolixsetnosy: + anacrolix
2012-03-14 00:56:52hayposetfiles: + threading_monotonic.patch

messages: + msg155703
2012-03-14 00:53:15hayposetfiles: + queue_monotonic.patch
keywords: + patch
messages: + msg155701
2012-03-08 00:38:57hayposetmessages: + msg155139
2012-03-07 22:40:26pitrousetnosy: + haypo, neologix, pitrou
messages: + msg155129
2012-03-07 19:38:39tasslehoffcreate