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

rpc.Conn doesn't support promise capabilities #2

Open
zombiezen opened this issue Sep 20, 2015 · 11 comments
Open

rpc.Conn doesn't support promise capabilities #2

zombiezen opened this issue Sep 20, 2015 · 11 comments
Assignees
Labels

Comments

@zombiezen
Copy link
Contributor

From @zombiezen on August 6, 2015 21:1

The CapDescriptor struct has a senderPromise field which rpc.Conn does not support, along with the corresponding Resolve message. The Go implementation does not emit this descriptor type, so it is only a concern if interoperating with another implementation that does.

Copied from original issue: zombiezen/go-capnproto#2

@zenhack
Copy link
Contributor

zenhack commented May 23, 2016

As you suspected, (I think) this is causing problems trying to do stuff with WebSession. I'm getting these errors on the console for irc-idler:

2016/05/23 17:44:18 rpc: unknown capability type senderPromise
sandstorm/supervisor.c++:1893: error: exception = capnp/rpc.c++:2643: disconnected: RpcSystem was destroyed.
stack: 0x44f1f0 0x4616c0 0x46de50 0x5e2fa0 0x5d0560 0x5cfe80
2016/05/23 17:44:18 rpc: aborted by remote: Peer did not implement required RPC message type.; (uint)message.which() = 2

@zombiezen
Copy link
Contributor Author

So a partial workaround is to use an error client in populateMessageCapTable. Unfortunately, this will break down if you need to actually use the capability (which I can't recall whether you need it or not). Does that work?

@zombiezen
Copy link
Contributor Author

The actual fix is a larger semantic change and I don't have the time to work on it right now.

@zenhack
Copy link
Contributor

zenhack commented May 24, 2016

My guess is that's not going to cut it, but I should do some more digging before I let you quote me on that. It's not what I'm hitting right this second, but it looks like streaming HTTP responses involves a promise, so without them I'd have to buffer the whole thing and send when the connection is closed. Haven't looked at the websocket calls, but my guess is that's not going to go well either.

Any pointers about what needs to change for a proper fix?

@zombiezen
Copy link
Contributor Author

Drat. It's been a while since I've looked at the spec, but generally:

  • You would need to create a new "wire promise" capability type
  • Handle the Resolve message in the coordinator goroutine in handleMessage
  • After a promise has resolved, if it is a capability that the sender has, then you will need to send a disembargo message and enqueue calls in an embargoClient until the embargo is lifted.

question.go will help you the most, but it the edge cases are quite subtle. I'm not entirely sure what the API should look like if Go were to send these, but if we don't come up with an API, proper integration testing would be difficult (since we'd have to rely on creating them in the wire format).

@kentonv
Copy link
Member

kentonv commented May 25, 2016

One easy way to work around this might be to treat senderPromise the same as senderHosted and ignore all Resolve messages. That will at least allow messages to be delivered to the promise. The down sides are:

  • Apps sometimes want to wait for promise resolution, and to find out if it resolved to an exception. You won't be able to provide that API. But, usually, it isn't needed.
  • If the promise resolves to a capability hosted on the receiver, messages sent to it will uselessly round-trip over the network rather than being delivered locally.

@kentonv
Copy link
Member

kentonv commented May 25, 2016

Correction: You shouldn't ignore Resolve but rather reply with unimplemented, reflecting the message back. I believe the C++ implementation will correctly drop the unused capability when it receives this.

@zombiezen
Copy link
Contributor Author

Neat. The unimplemented logic should already be there, so maybe it is just a simple matter of hacking those cases to be treated identically. Thanks @kentonv!

zenhack added a commit to zenhack/go-capnp that referenced this issue Jul 13, 2016
The workaround is described in a comment included in the patch.
zombiezen added a commit that referenced this issue Jul 31, 2016
Workaround for #2.  Sender promises now no longer error the connection.
Thanks, Ian!
@zombiezen zombiezen modified the milestone: v3 May 16, 2017
@zenhack
Copy link
Contributor

zenhack commented Apr 3, 2018

Looks like the v3 branch has regressed on this; recvCap doesn't have a case for senderPromise. Any reason not to implement the same workaround there as we did for the v2 branch, in the interm?

@zombiezen
Copy link
Contributor Author

Same workaround could apply. That was the next thing I was going to tackle, but it's looking like I won't be able to get back to v3 for a while.

zenhack added a commit to zenhack/haskell-capnp that referenced this issue Jan 23, 2019
Worth noting that the previous behavior isn't broken; it's the same as a
workaround that the Go implementation uses in lieu of proper promise
resolution support, see:

    capnproto/go-capnp#2 (comment)

...but actually supporting resolve is better of course.
@zenhack
Copy link
Contributor

zenhack commented Jun 14, 2022

I think this probably doesn't need to be on the v3 milestone; it's all internals, so shouldn't require breaking compatibility to address. Any objections to removing it?

@zenhack zenhack removed this from the 3.0 milestone Jul 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants