[issue11757] test_subprocess.test_communicate_timeout_large_ouput failure on select(): negative timeout?
Changes by Charles-François Natali cf.nat...@gmail.com: -- status: pending - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11757 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11757] test_subprocess.test_communicate_timeout_large_ouput failure on select(): negative timeout?
Christian Heimes added the comment: Is there anything left to do here? -- nosy: +christian.heimes resolution: - fixed stage: needs patch - committed/rejected status: open - pending ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11757 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11757] test_subprocess.test_communicate_timeout_large_ouput failure on select(): negative timeout?
Reid Kleckner r...@mit.edu added the comment: I think the best behavior would be to go ahead and check one last time before raising the exception, so _remaining_time should turn a negative value into 0 (assuming that a timeout value of zero does the right thing for our use case). If people don't feel that is best, refactoring _remaining_time to incorporate the check in _check_timeout would also be good. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11757 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11757] test_subprocess.test_communicate_timeout_large_ouput failure on select(): negative timeout?
Charles-Francois Natali neolo...@free.fr added the comment: Oh, I didn't know. In this case, is my commit 3664fc29e867 correct? I think that it is, because without the patch, subprocess may call poll() with a negative timeout, and so it is no more a timeout at all. Yes, it looks correct. But I think there are a couple places left where functions can be called with a negative timeout, for example here : 1537 stdout, stderr = self._communicate_with_select(input, endtime, 1538 orig_timeout) 1539 1540 self.wait(timeout=self._remaining_time(endtime)) or here: 1113 if self.stdout is not None: 1114 self.stdout_thread.join(self._remaining_time(endtime)) 1115 if self.stdout_thread.isAlive(): Also, it might be simpler and cleaner to factorize the raising of the TimeoutExpired exception inside _remaining_time, instead of scattering this kind of checks around the file: 1514 remaining = self._remaining_time(endtime) 1515 if remaining = 0: 1516 raise TimeoutExpired(self.args, timeout) merging what's done in _check_timeout -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11757 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11757] test_subprocess.test_communicate_timeout_large_ouput failure on select(): negative timeout?
Roundup Robot devnull@devnull added the comment: New changeset 3982be773b54 by Antoine Pitrou in branch 'default': Issue #11757: select.select() now raises ValueError when a negative timeout http://hg.python.org/cpython/rev/3982be773b54 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11757 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11757] test_subprocess.test_communicate_timeout_large_ouput failure on select(): negative timeout?
STINNER Victor victor.stin...@haypocalc.com added the comment: Le vendredi 08 avril 2011 à 05:34 +, Charles-Francois Natali a écrit : Charles-Francois Natali neolo...@free.fr added the comment: You may also patch poll_poll(). Poll accepts negative timeout values, since it's the only way to specify an infinite wait (contrarily to select which can be passed NULL). Oh, I didn't know. In this case, is my commit 3664fc29e867 correct? I think that it is, because without the patch, subprocess may call poll() with a negative timeout, and so it is no more a timeout at all. If I am correct, it is a real bug. Should it be fixed in Python 2.7, 3.1 and 3.2? ... Hum, it looks like communicate() timeout was introduced in Python 3.3: c4a0fa6e687c. This commit has no reference to an issue: it is the issue #5673. And as it was already written in msg130851, the doc is wrong: the doc indicates that the feature was introduced in 3.2, but it is 3.3 only. The change is not documented in Misc/NEWS. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11757 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11757] test_subprocess.test_communicate_timeout_large_ouput failure on select(): negative timeout?
Charles-Francois Natali neolo...@free.fr added the comment:
It seems to have fixed the failure, no ?
I don't know what's the policy regarding syscall parameters check, but
I think it'd be better to check that the timeout passed to select is
not negative, and raise an exception otherwise, instead of silently
storing it into struct timeval (with an overflow) before passing it to
select.
Attached is a patch + test that does just that.
--
keywords: +patch
Added file: http://bugs.python.org/file21566/select_negative_timeout.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue11757
___diff -r bbfc65d05588 Lib/test/test_select.py
--- a/Lib/test/test_select.py Thu Apr 07 10:48:29 2011 -0400
+++ b/Lib/test/test_select.py Thu Apr 07 21:06:59 2011 +0200
@@ -20,6 +20,7 @@
self.assertRaises(TypeError, select.select, [self.Nope()], [], [])
self.assertRaises(TypeError, select.select, [self.Almost()], [], [])
self.assertRaises(TypeError, select.select, [], [], [], not a number)
+self.assertRaises(ValueError, select.select, [], [], [], -1)
def test_returned_list_identity(self):
# See issue #8329
diff -r bbfc65d05588 Modules/selectmodule.c
--- a/Modules/selectmodule.cThu Apr 07 10:48:29 2011 -0400
+++ b/Modules/selectmodule.cThu Apr 07 21:06:59 2011 +0200
@@ -234,6 +234,11 @@
timeout period too long);
return NULL;
}
+if (timeout 0) {
+PyErr_SetString(PyExc_ValueError,
+timeout must be non-negative);
+return NULL;
+}
seconds = (long)timeout;
timeout = timeout - (double)seconds;
tv.tv_sec = seconds;
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11757] test_subprocess.test_communicate_timeout_large_ouput failure on select(): negative timeout?
Gregory P. Smith g...@krypto.org added the comment: Adding that check with an exception to selectmodule.c is a good idea. i like your patch. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11757 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11757] test_subprocess.test_communicate_timeout_large_ouput failure on select(): negative timeout?
STINNER Victor victor.stin...@haypocalc.com added the comment: You may also patch poll_poll(). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11757 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11757] test_subprocess.test_communicate_timeout_large_ouput failure on select(): negative timeout?
Charles-Francois Natali neolo...@free.fr added the comment: You may also patch poll_poll(). Poll accepts negative timeout values, since it's the only way to specify an infinite wait (contrarily to select which can be passed NULL). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11757 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11757] test_subprocess.test_communicate_timeout_large_ouput failure on select(): negative timeout?
Changes by STINNER Victor victor.stin...@haypocalc.com: -- title: test_subprocess failure - test_subprocess.test_communicate_timeout_large_ouput failure on select(): negative timeout? ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11757 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11757] test_subprocess.test_communicate_timeout_large_ouput failure on select(): negative timeout?
Roundup Robot devnull@devnull added the comment: New changeset 3664fc29e867 by Victor Stinner in branch 'default': Issue #11757: subprocess ensures that select() and poll() timeout = 0 http://hg.python.org/cpython/rev/3664fc29e867 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11757 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
