Message232174
On Thu, Dec 4, 2014 at 7:21 PM, Yury Selivanov <report@bugs.python.org>
wrote:
>
> Yury Selivanov added the comment:
>
> > - modify tests to set the event loop to the newly created event loop,
> instead of setting it to None
>
> I'm not sure that this particular change is a great idea. I kind of liked
> that unittests ensure that loop is passed everywhere explicitly in asyncio.
>
Indeed, don't break that.
>
> > - modify get_event_loop() to always raise a RuntimeError if the thread
> has no event loop. Before an AssertionError was not raised if python runs
> with -O option
>
> +1.
>
+1
>
> > - modify BaseEventLoop._assert_is_current_event_loop() to fail if the
> thread has an event loop
>
> Hm, I think I don't understand why this function doesn't to anything when
> there is no loop in the thread... Let's fix it. In your patch, you should
> also replace 'except AsserionError' with 'except RuntimeError'.
>
>
> > Does it sound reasonable to raise an exception if an event loop is used
> from the wrong thread?
>
> I think we should in debug mode at least.
>
>
> > Getting thread ID or current thread may be expensive.
>
> Victor, can you benchmark this? I'm pretty sure that Guido is right about
> this, but sometimes syscalls are pretty fast. I doubt that a lot of people
> know about debug mode, so if it has a negligible cost I'd do the check
> every time.
>
I take it back, it's only 250 nsec to call threading.current_thread(), and
300 nsec to acquire and release a lock.
I still think you shouldn't set the thread when the loop is created but
only when run*() is active. (Using call_later() from another thread when
the loop is inactive should not be an error IMO -- there's nothing against
passing a selector instance to another thread as long as only one thread at
a time waits for it.) |
|
Date |
User |
Action |
Args |
2014-12-05 03:30:49 | gvanrossum | set | recipients:
+ gvanrossum, vstinner, yselivanov |
2014-12-05 03:30:48 | gvanrossum | link | issue22926 messages |
2014-12-05 03:30:48 | gvanrossum | create | |
|