Both, please. The googoe code issue is to discuss the problem, the Rietveld is to review the patch.
On Mon, Jan 20, 2014 at 8:06 AM, Gustavo Carneiro <[email protected]> wrote: > On 20 January 2014 15:59, Guido van Rossum <[email protected]> wrote: >> >> Yup, I think you've found a bug. wait_for() should definitely cancel >> the thing it's waiting for -- if you don't want that, you should call >> wait_for(shield(<coroutine_or_task>), <timeout>). >> >> It's likely that most of the wait_for() (or its unittests :-) >> implementation predates the rethinking of cancellation. >> >> Can you help fixing this before Python 3.4 beta 3 goes out 9Jan 26 I >> think)? I'm really busy through the 31st so I need help. > > > Sure, I think I can fix this. How is the preferred way to submit patches > these days, codereview or googlecode issue, or both? > > Thanks. > >> On Mon, Jan 20, 2014 at 7:41 AM, Gustavo Carneiro <[email protected]> >> wrote: >> > Hi. >> > >> > I spent a few hours chasing a nasty bug in either tulip or my code. I >> > got >> > this traceback: >> > >> > Traceback (most recent call last): >> > File "/home/gjc/projects/mollytest/tests/base.py", line 63, in runTest >> > return loop.run_until_complete(self.runTestTask()) >> > File "lib/asyncio/base_events.py", line 177, in run_until_complete >> > return future.result() >> > File "lib/asyncio/futures.py", line 236, in result >> > raise self._exception >> > File "lib/asyncio/tasks.py", line 279, in _step >> > result = coro.send(value) >> > File "tests/450-api-place-hide-exchange.py", line 42, in runTestTask >> > lambda line: line[:2] == ['betslip_account_added', '3'] and line[5] >> > == >> > 'dummyex')), >> > File "tests/450-api-place-hide-exchange.py", line 35, in wait_for_line >> > line = yield from api.read_async_line() >> > File "/home/gjc/projects/mollytest/mollyapiclient.py", line 68, in >> > read_async_line >> > data = yield from read >> > File "lib/asyncio/streams.py", line 321, in readline >> > assert self._waiter is None >> > AssertionError >> > >> > After much debugging I discovered that: >> > >> > 1. That assertion fails when there are two concurrent StreamReader >> > read >> > operations; >> > >> > 2. I eventually found out that the "previous" running read operation >> > was >> > this one: >> > >> > # self.async_reader is a StreamReader object >> > data = yield from asyncio.wait_for(self.async_reader.readline(), >> > timeout) >> > >> > In the above code, if the timeout actually happens, then >> > self.async_reader.readline(), which has been converted to a Task by >> > asyncio.wait_for(), is left running. After the timeout, my script keeps >> > running, but the next call to StreamReader.readline() will fail and >> > generate >> > the above traceback because the previous Task is still running. I >> > confirmed >> > this by adding a "fut.cancel()" call to tasks.wait_for() right before >> > "raise >> > futures.TimeoutError()", which made the problem go away. >> > >> > I was a bit surprised by the behaviour of wait_for(). If wait_for() >> > creates >> > a Task, I reckon it should be the one to cancel it when the timeout >> > happens. >> > If you think shouldn't cancel a Task, then I think wait_for() shouldn't >> > create it in the first place, and so it should accept only Task as >> > parameter, and not coroutine (generator): in this case it's the >> > application >> > programmer who should take care of cancelling the task on timeout. >> > >> > Thoughts? >> > >> > -- >> > Gustavo J. A. M. Carneiro >> > Gambit Research LLC >> > "The universe is always one step beyond logic." -- Frank Herbert >> >> >> >> -- >> --Guido van Rossum (python.org/~guido) > > > > > -- > Gustavo J. A. M. Carneiro > Gambit Research LLC > "The universe is always one step beyond logic." -- Frank Herbert -- --Guido van Rossum (python.org/~guido)
