Message231810
call_soon_after_close.diff looks good but also incomplete: call_later, call_at and run_in_executor should also raise an exception if the event loop is closed.
You should also add call_soon_threadsafe to the test.
> So for consistency this patch should grow quite a bit (unless create_connection, add_reader etc. already raise in this case).
Yes, add_reader raises an exception and there is an unit test to ensure that.
create_task() is a little bit special, it does not schedule immediatly a coroutine. IMO it makes sense to fail if the loop was closed, I don't see how the task can be executed if the loop was executed, so calling create_task() on a closed loop looks like a bug.
I suggest to modify the following methods:
- call_soon, call_soon_threadsafe
- call_at, call_later
- run_in_executor
- create_task
- add_signal_handler
- subprocess_exec, subprocess_shell
Did I miss something?
I'm not sure that *all* asyncio methods should must call _check_closed(), it may make the code harder to read. If the basic functions like call_soon and add_reader already handle the closed status, I guess that all other methods will fail loudly, so they don't need to be modified.
For example, stop() calls call_soon() and so will also raises an exception if the loop is closed.
Maybe we should add almost all methods to the test checking that calling these methods on a closed loop fail. |
|
Date |
User |
Action |
Args |
2014-11-28 12:48:44 | vstinner | set | recipients:
+ vstinner, gvanrossum, torsten, yselivanov |
2014-11-28 12:48:44 | vstinner | set | messageid: <1417178924.08.0.875675983374.issue22922@psf.upfronthosting.co.za> |
2014-11-28 12:48:44 | vstinner | link | issue22922 messages |
2014-11-28 12:48:43 | vstinner | create | |
|