Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

steal Dave's idea of having a special exception for concurrent calls to send/recv #144

Closed
njsmith opened this issue Apr 29, 2017 · 5 comments

Comments

@njsmith
Copy link
Member

njsmith commented Apr 29, 2017

He uses ResourceBusy, with ReadResourceBusy and WriteResourceBusy subclasses.

Not sure if we need to distinguish between read/write versions, but switching to ResourceBusy at least would be a nice change.

@dabeaz
Copy link

dabeaz commented Apr 30, 2017

Just a note about this... these exceptions are raised when an operation is attempted on a resource that's in use. However, there is also a related task.interrupt() method that causes a TaskInterrupted() exception to be raised in whatever blocking operation is in progress. It's kind of like cancellation, but not really cancellation (e.g., I'm just booting you off of that operation, go try again later.).

I'm not 100% sure this is fully fleshed out in Curio at the moment, but my thinking on this is that if a resource is busy, you'll at least get an exception about it (e.g., ReadResourceBusy). How a task goes about handling that is somewhat open-ended--it could simply retry later. However, if it wanted to do it, it could also explicitly yank the other task off by calling task.interrupt() on it. All of this would require some kind of careful coordination on the part of the user in both tasks. I wasn't planning to make Curio do anything all on its own in this regard. Instead, I figured that if there's going to be a mechanism to report an error, there should at least be a mechanism, maybe a bit more refined than outright cancellation, to recover from it.

@njsmith
Copy link
Member Author

njsmith commented Apr 30, 2017

Huh. I did see task.interrupt, but hadn't made the connection to ResourceBusy exceptions. That's interesting. Sort of like a "hey, wake up and pay attention!" primitive.

I'm not quite thinking of when this would be useful, and it seems odd that the interrupt is addressed by task while the error is addressed by operation? Though I think I can see how to make it addressed by operation. (Oh, that's what that "priority" stuff you had and then removed was, wasn't it. Now it all makes sense 😁)

I guess one use case would be implementing blocking primitives that can be woken in multiple circumstances. I needed that for some happy eyeballs code I wrote a few days ago (not yet online but happy to share if anyone's curious), and I did it by having the blocking task set up a cancel scope and then leave it where the waking task could find it. Differences between this and task.interrupt:

  • Cancel scope requires some preemptive cooperation from the blocking task to set up the scope. (Though in practice I guess the blocking task always has to make a record somewhere so the waking task can find it!)
  • Detailed semantics are obviously different: task.interrupt is a single edge-triggered interrupt wherever a task might be; cancelling a scope is a level-triggered interrupt but only inside the scope. (In my happy eyeballs prototype I don't even bother making a note when the blocking task finishes the blocking operation normally – if the other task tries to interrupt it late then it has no effect because the scope is closed anyway.)
  • Obviously it's nice to reuse an existing concept (cancellation) if you can, instead of having two (cancellation and interrupt). But this only happens because trio has a much more elaborate cancellation system than curio :-). In curio's case, task.cancel and task.interrupt feel much more like an idiomatic pair.

.... Anyway,my main motivation for opening this issue is that I've been elaborating trio's "abstract steam interface" and implementing multiple "socket-like" objects (see trio/abc.py in the SSL PR if you're curious), and one of the things I'm noticing is that I really do want the same "error if someone else is sending/receiving" semantics everywhere, and that it would be convenient if there were a standard, documented way that everyone should implement it. 😁 This feels easier if there's a custom-built exception instead of saying everyone should use RuntimeError.

@dabeaz
Copy link

dabeaz commented May 1, 2017

Honestly, I'm not even sure I'm going to keep that interrupt thing. At one point, I did have this priority mechanism, but after some further thought, didn't like the idea of putting that much implicit magic into the kernel. Instead, I pushed it out to "user space". A task will get an exception if there's a conflict. It can interrupt the other waiting task if it wants. Beyond that, the handling is "someone else's problem." I suppose a higher level Streams API could incorporate something more advanced around this, but from the kernel point of view, I wanted to keep it simple.

@njsmith
Copy link
Member Author

njsmith commented May 1, 2017

If I were going to implement the priority thing in trio, I wouldn't put it in the kernel-equivalent, I'd do something like:

class socket:
    ...
    async def sendall(self, data, priority=0):
        async with self._resource_guard(priority=priority):
            ...

and have that instance object keep track of who's currently using the resource and implementing the kick-them-out-or-not logic. Just wrap one of those around every "resource". (And the kernel would still need to raise some error if two tasks still managed to block on the same resource at the same time, but that could be like an AssertionError or something.)

I don't think it'd be too complicated... something like:

class ResourceGuard:
    def __init__(self):
        self._current_cancel_scope = None
        self._current_priority = None
        self._lock = trio.Lock()

    @acontextmanager
    async def __call__(self, priority):
        if self._current_priority is not None and self._current_priority >= priority:
            raise ResourceBusy  # they win
        else:
            # either no-one has it, or a low priority task has it. either way we're taking it.
            if self._current_cancel_scope is not None:
                self._current_cancel_scope.cancel()
            self._current_task_priority = priority
            try:
                with trio.open_cancel_scope() as self._current_cancel_scope:
                    async with self._lock:  # wait until they're out of the way
                        yield
                if self._current_cancel_scope.cancelled_caught:
                   raise ResourceStolen
            finally:
                self._current_priority = None
                self._current_cancel_scope = None

I think this works correctly even in tricky cases like if priority=2 task comes along while a priority=1 task is waiting to steal the resource from a priority=0 task. I'm not sure how easy it would be to translate to curio, though – it kinda relies on stuff like cancel scopes, and cancel() not blocking, and it might be difficult to keep everything consistent if curio wants to put more cancellation points in there (I'm not sure if it does or not).

It's a nice exercise in any case :-)

BTW the happy eyeballs code I mentioned above is now in #145, or here's a direct link to the current version: https://github.com/njsmith/trio/blob/23c02cb4a070fe43ee6655a296995dc317102852/notes-to-self/happy-eyeballs.py

@njsmith
Copy link
Member Author

njsmith commented Jun 12, 2017

#107 adds a ResourceBusyError and uses it for these cases.

@njsmith njsmith closed this as completed Jun 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants