Cap'n'proto premature destruction of interface?

Question

Let's say I have a streaming API like this:

interface MyInterface {
  addListener @0 (listener: Listener) -> (registration: RegisteredListener);

  interface Listener {
    update @0 () -> stream;
  }
  interface RegisteredListener {
  }
}

I'm having a challenge where the destructor for the server implementation of MyInterface is running before the last registered interface has been released. How can I properly communicate the relationship to cap'n'proto RPC so that even if the client releases it's MyInterface::Client before the RegisteredListener::Client, that either MyInterface::Server lifetime is extended or the RegisteredListener::Server is smart enough to recognize that the original server instance its tracking the registration for is dead. Alternatively, am I misusing the API in some basic way?

Roughly the C++ code looks like this. Server:

class MyInterfaceImpl : public MyInterface {
 class Registered final : public MyInterface::RegisteredListener::Server {
  public:
   Registered(MyInterfaceImpl* parent, uint64_t registrationId) : parent_(parent), id_(registrationId) {}

   ~Registered() {
     parent->unregister(id_);
   }
  private:
   MyInterfaceImpl *parent_;
   uint64_t id_;
 };

 public:
  kj::Promise<void> addListener(MyInterface::AddListenerContext context) {
    auto registrationId = ++registrationId_;
    clients_.emplace_back(context.getParams().getListener());
    registrations_.emplace_back(registrationId);
    context.getResult().setRegistration(kj::heap<Registered>(this, registrationId));
    return kj::READY_NOW;
  }

  void unregister(uint64_t registrationId) {
    auto found = std::find(registrations_.begin(), registrations_.end(), registrationId);
    clients_.erase(clients_.begin() + (found - registrations_.begin()));
  }

 private:
  std::vector<MyInterface::Listener::Client> clients_;
  std::vector<uint64_t> registrations_;
  uint64_t registrationId_ = 0;
};

The client code looks something like this:

capnp::EzRpcClient client("localhost:5923");
MyInterface::Client intf = client.getMain<MyInterface>();
auto& waitScope = client.getWaitScope();

auto listenerRequest = intf.addListenerRequest();
auto listenerPromise = listenerRequest.send();
listenerPromise.wait(waitScope);

{
  auto listenerRequest2 = intf.addListenerRequest();
  auto listenerPromise2 = listenerRequest2.send();
  listenerPromise2.wait(waitScope);
}

Since this is all single-threaded it's pretty easy to spot the use-after-free. I put debug statements in ~MyInterfaceImpl and ~RegisteredListener and the second listener is getting unregistered after ~MyInterfaceImpl. I know that my add listener requests don't have the actual client object but I'm hoping that's not actually an important detail for this.

Answer 1

I recommend having Registered class hold a MyInterface::Client pointing to the parent, in addition to the plain pointer it currently holds.

MyInterfaceImpl *parent_;
MyInterface::Client ownParent_;
uint64_t id_;

This ensures that as long as a Registered still exists, the MyInterfaceImpl won't be destroyed.

Inside the implementation of addListener(), you can obtain a MyInterface::Client pointing to this by calling thisCap().

context.getResult().setRegistration(kj::heap<Registered>(
    this, thisCap(), registrationId));