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

Allow cancellation of all capability calls #343

Merged
merged 1 commit into from
Nov 8, 2023

Conversation

LasseBlaauwbroek
Copy link
Contributor

@LasseBlaauwbroek LasseBlaauwbroek commented Oct 28, 2023

This patch allows the cancellation of all RPC server calls without a possibility to opt-out. I see no reason in Python code why cancelling a call should not be allowed.

My understanding is that in the C++ code, cancellation is not enabled by default because it poses some kind of security risk. @kentonv is there reason to believe that cancellation poses a risk here? Or can we safely enable it?

Patch is compatible with 1.0 and pre-1.0.

@LasseBlaauwbroek LasseBlaauwbroek changed the title Allow cancellation of all capability contexts Allow cancellation of all capability calls Oct 29, 2023
@kentonv
Copy link
Member

kentonv commented Oct 30, 2023

There's a risk that applications might not be designed to expect cancellation and may be buggy when it happens. For example, maybe an application takes some sort of a lock on an internal data structure, and then the request is canceled, so the lock is never released. A malicious client could make this happen intentionally to DOS the server.

That said, in retrospect, in C++ at least, I wish we had always enabled cancellation by default. In KJ C++ code, we are always using RAII design which is naturally cancellation-safe, and we've had way more bugs from cancellation being blocked by default than from cancellation being unexpected. That's why 1.0 changed to making cancellation be an annotation that can be applied across a whole file at once, so it's easy to just opt in everywhere. In 2.0, we might even change the default.

In non-RAII languages, though, the calculus might be a bit different; I'm not sure whether you can really expect Python application code to be cancellation-safe naturally. I guess this is up to you to decide, though.

@LasseBlaauwbroek
Copy link
Contributor Author

LasseBlaauwbroek commented Oct 31, 2023

Thanks @kentonv.

Python cancellation works quite a bit different from KJ cancellation. When a promise is canceled in Python, a CancellationException is thrown into the code. This usually terminates the running code, but it can also be caught, and cleanup can be run. For example, when you have to acquire a lock, idiomatic Python code looks like this:

with lock:
    <code that needs the lock>

This desugars to

try:
    lock.acquire()
    <code that needs the lock>
finally:
    lock.release()

The finally clause is executed even when the cancellation happens. It is even possible to completely "cancel" a cancellation by suppressing the CancellationException.

I've tested that all this works in the context of Pycapnp and the KJ even loop.

As such, I vote that cancellation can indeed be safely enabled.

@LasseBlaauwbroek
Copy link
Contributor Author

@haata what do you think about this?

@haata
Copy link
Collaborator

haata commented Nov 8, 2023

I think this is reasonable. In the python world you never know what's going to happen after an exception is caught (and there's not much you can do about it if you need to force certain behaviours).

@haata haata merged commit 0ec4d0b into capnproto:master Nov 8, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants