[Python-ideas] Enabling Event.set to notify all waiters with an exception

[Nathaniel Smith]

On Tue, Jul 18, 2017 at 2:33 PM, Pau Freixes <pfreixes at gmail.com> wrote:
> Morning guys,

(Not everyone here is a guy.)

> I came across with that idea trying to resolve a typical dogpile
> pattern [1], having many DNS calls to the same domain because of a
> miss in a DNS cache.
>
> The usage of the the set either to notify that the waiters could be
> awake and get the result from the cache or use it to notify that
> something was wrong helped me to reduce the complexity of the code.
> Just as an example:
>
>  ```
> if key in throttle_dns_events:
>     yield from throttle_dns_events[key].wait()
> else:
>     throttle_dns_events[key] = Event(loop=loop)
>     try:
>         addrs = yield from \
>             resolver.resolve(host, port, family=family)
>         cached_hosts.add(key, addrs)
>         throttle_dns_events[key].set()
>     except Exception as e:
>         # any DNS exception, independently of the implementation
>         # is set for the waiters to raise the same exception.
>         throttle_dns_events[key].set(exc=e)
>         raise
>     finally:
>         throttle_dns_events.pop(key)
> ```
>
> Any error caught by the locker will be broadcasted to the waiters. For
> example, a invalid hostname.
>
> I tried to open a PR to the CPython implementation, and they claim
> that the current interface of all of the locks objects behind the
> asyncio.locks [2] module try to keep the same interface as the
> threading one [3]. Therefore, to modify the asyncio implementation
> would need first a change in the threading interface.
>
> I was determined to justify that change, but after a bit research, I
> didn't find any example in other languages such as Java [4], C# [5] or
> C++ [6]  allowing you to send an exception as a signal value to wake
> up the sleeping threads.

'Event' is designed as a lowish-level primitive: the idea is that it
purely provides the operation of "waiting for something", and then you
can compose it with other data structures to build whatever
higher-level semantics you need. From this point of view, it doesn't
make much sense to add features like exception throwing -- that would
make it more useful for some particular cases, but add overhead that
others don't want or need.

In this case, don't you want to cache an error return as well, anyway?

It sounds like you're reinventing the idea of a Future, which is
intended as a multi-reader eventually-arriving value-or-error --
exactly what you want here. So it seems like you could just write:

# Conceptually correct, but subtly broken due to asyncio quirks
if key not in cache:
    cache[key] = asyncio.ensure_future(resolver.resolve(...))
return await cache[key]

BUT, unfortunately, this turns out to be really broken when combined
with asyncio's cancellation feature, so you shouldn't do this :-(.
When using asyncio, you basically need to make sure to never await any
given Future more than once.

Maybe adding a shield() inside the await is the right solution? The
downside is that you actually do want to propagate the cancellation
into the resolution Task, just... only if *all* the callers are
cancelled *and* only if you can make sure that the cancellation is not
cached. It's quite tricky actually!

But I don't think adding exception-throwing functionality to Event()
is the right solution :-)

-n

-- 
Nathaniel J. Smith -- https://vorpus.org