capnproto / go-capnp

Cap'n Proto library and code generator for Go
https://capnproto.org
Other
1.23k stars 111 forks source link

Passing last reference for a client to a method call on that client causes a deadlock. #214

Open zenhack opened 2 years ago

zenhack commented 2 years ago

There's a deadlock footgun that I hit while writing tests for #213. Roughly, if you do:

foo.Method(ctx, func(p Foo_method_Params) error {
  p.SetFoo(foo)
  return nil
})

...on a remote capability foo, and there are no other reference to foo, you get a deadlock, because after constructing the cap table for the message:

  1. The rpc subsystem assumes the Params struct owns the reference you just passed it, so wants to decrement it.
  2. Doing so drops its refcount to 0, so it calls .Release() on the client.
  3. .Release() waits for there to be no outstanding calls, which will never happen, because it was called by code sending foo.Method.

This is arguably a bug in the application, the solution to which is just to do p.SetFoo(foo.AddRef()). But it's sad that the failure mode for this mistake is a deadlock; it would be nice to try to detect this and panic or something instead.

zenhack commented 2 years ago

I think we could do a check for this inside Client.SendCall, just by looping over the CapTable and checking that nothing is pointer-equal to the receiver.