Tim Golden added the comment:
+1 for Kristjan's latest patch. And thanks for working this through, Kristjan:
I'd missed the fact that the microseconds conversion could itself overflow even
an unsigned long.
--
___
Python tracker
Kristján Valur Jónsson added the comment:
Thanks. Can you confirm that it resolves the issue? I'll get it checked in
once I get the regrtest suite run.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20737
Tim Golden added the comment:
s/Py_LONG_LONG/PY_LONG_LONG/
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20737
___
___
Python-bugs-list mailing
Kristján Valur Jónsson added the comment:
I see, I wasn't able to compile it yesterday when I did it :)
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20737
___
Tim Golden added the comment:
I can confirm that the attached test.py times out after 2150 seconds (ie
30+ minutes) with your (tweaked) patch applied:
python test.py 2150
Running Debug|Win32 interpreter...
2014-05-08 10:33:53.670091
Expected to time out by 2014-05-08 11:09:43.670091
Timed Out
Kristján Valur Jónsson added the comment:
nope, let's not do that :)
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20737
___
___
Roundup Robot added the comment:
New changeset 80b76c6fad44 by Kristján Valur Jónsson in branch 'default':
The PyCOND_TIMEDWAIT must use microseconds for the timeout argument
http://hg.python.org/cpython/rev/80b76c6fad44
--
nosy: +python-dev
___
Roundup Robot added the comment:
New changeset ab5e2b0fba15 by Kristján Valur Jónsson in branch '3.3':
The PyCOND_TIMEDWAIT must use microseconds for the timeout argument
http://hg.python.org/cpython/rev/ab5e2b0fba15
New changeset 7764bb7f2983 by Kristján Valur Jónsson in branch '3.4':
Merging
Changes by Kristján Valur Jónsson krist...@ccpgames.com:
--
resolution: - fixed
status: open - closed
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20737
___
Tim Golden added the comment:
The attached patch uses DWORD (essentially: unsigned long) in
condvar.h:PyCOND_TIMEDWAIT.
Adding Kristjan as it was his code.
--
keywords: +patch
nosy: +kristjan.jonsson
Added file: http://bugs.python.org/file35169/issue20737.condvar.patch
Changes by Tim Golden m...@timgolden.me.uk:
--
stage: - patch review
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20737
___
___
Python-bugs-list
Kristján Valur Jónsson added the comment:
changing long to DWORD doesn't really fix the overflow issue.
The fundamental problem is that some of the apis, e.g. WaitForSingleObject have
a DWORD maximum. so, we cannot support sleep times longer than some particular
time.
Microseconds was chosen
Tim Golden added the comment:
Thanks for the feedback, Kristjan. You're obviously correct in that we
can't account for timeouts greater than DWORD-size milliseconds and your
proposed solution looks reasonable.
However, I'd like to close off *this* particular issue which turns on
the implicit
R. David Murray added the comment:
An implicit ceiling of 4000 seconds on the timeout? I routinely use timeouts
of approximately 24 hours in calls to Event().wait(). What am I
misunderstanding? If I'm not misunderstanding, then no, I don't think that
change would be acceptable.
--
Kristján Valur Jónsson added the comment:
Tim, how about changing the variable to unsigned long? I'd like the
signature of the function to be the same for all platforms.
This will change the code and allow waits for up to 4000 seconds.
There is still an overflow problem present, though.+
R. David Murray added the comment:
I have production code on Windows using python2.7 that calls Event().wait()
with a timeout of approximately 24 hours, and it works just fine. Having that
no longer work is, IMO, an unacceptable regression. (I'm ready to move this
code to python3 as soon as
Tim Golden added the comment:
Just to be clear: the change *I'm* proposing for this issue has nothing
to do with limiting the wait, artificially or otherwise. It's simply
undoing an unintended conversion from unsigned to signed and back again,
whicih currently causes any wait of more than
Tim Golden added the comment:
Updated patch with unsigned long applied throughout
--
Added file: http://bugs.python.org/file35173/issue20737.condvar.2.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20737
Kristján Valur Jónsson added the comment:
Hi there.
When I said 4000, that was because of the conversion to microseconds which
happens early on.
I'm not trying to be difficult here Tim, it's just that you've pointed out a
problem and I'd like us to have a comprehensive fix.
unsigned long,
R. David Murray added the comment:
I'm adding Mark Dickinson to nosy to see if our math expert has a comment on
the arithmetic :)
--
nosy: +mark.dickinson
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20737
Kristján Valur Jónsson added the comment:
(cont.)
so, I suggest that we modify the API to use Py_LONG_LONG usec
Does that sound reasonable?
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20737
Kristján Valur Jónsson added the comment:
Ah, I saw this code here in thread_nt.h:
if ((DWORD) milliseconds != milliseconds)
Py_FatalError(Timeout too large for a DWORD,
please check PY_TIMEOUT_MAX);
the PyCOND_TIMEDWAIT is currently only used by the GIL
Kristján Valur Jónsson added the comment:
Here is a proposed alternative patch.
No additional checks, just a wider Py_LONG_LONG us wide enough to accommodate
32 bits of milliseconds as before.
--
Added file: http://bugs.python.org/file35175/condwait.patch
Kristján Valur Jónsson added the comment:
fix patch, was using git format
--
Added file: http://bugs.python.org/file35176/condwait.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20737
Tim Golden added the comment:
In thread_nt.h:EnterNonRecursiveMutex the millisecond version (here: 2148000)
of the seconds you passed in are converted to microseconds (so: 214800).
This is then passed to condvar.h:PyCOND_TIMEDWAIT which expects a long, whose
32-bit limit is 2147483647. So
Changes by Alex Grönholm alex.gronholm+pyt...@nextday.fi:
--
nosy: +alex.gronholm
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20737
___
___
New submission from raruler:
I've tried this with both the 32-bit and 64-bit versions of 3.3.4 on two
Windows 7 x64 machines.
threading.Event().wait(2148) and a lock obtained from _thread used as
lock.acquire(True, 2148) will never return. Anything under 2148 seems to work
just fine, but
Changes by Antoine Pitrou pit...@free.fr:
--
nosy: +brian.curtin, tim.golden
versions: +Python 3.4
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20737
___
Changes by R. David Murray rdmur...@bitdance.com:
--
nosy: +r.david.murray
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20737
___
___
29 matches
Mail list logo
