capnproto / pycapnp

Cap'n Proto serialization/RPC system - Python bindings
BSD 2-Clause "Simplified" License
458 stars 125 forks source link

Allow cancellation of all capability calls #343

Closed LasseBlaauwbroek closed 8 months ago

LasseBlaauwbroek commented 8 months ago

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.

kentonv commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

@haata what do you think about this?

haata commented 8 months ago

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).