Handle incoming resolve messages.

[zenhack]

Still TODO:

• We need to handle disembargos with target = (importedCap = ...).
• Testing.

[zenhack]

Alright, this is done.

This basically completes the promise resolution detour from 3PH, Though there's a semantic thing that I don't like, which is that Fulfill(client) feels like it should take ownership of client, but doesn't. I may submit another PR changing this before moving on.

[lthibault]

I'm very much in favor of taking the time to make reference ownership semantics consistent. 👍

[lthibault]

LGTM.

[lthibault]

Test is timing out.

Re-running.

[zenhack]

Note that there does appear to be a real deadlock that needs to be addressed; the test failures are not spurious.

[zenhack]

Status update: there are at least two bugs, one which is a deadlock, and another that is a message-reordering problem.

The deadlock is "solved" by this change to the test:

diff --git a/rpc/senderpromise_test.go b/rpc/senderpromise_test.go
index 3bcaec4..7cb5ea2 100644
--- a/rpc/senderpromise_test.go
+++ b/rpc/senderpromise_test.go
@@ -392,6 +392,7 @@ func TestPromiseOrdering(t *testing.T) {
 		if i == 100 {
 			go func() {
 				bs := testcapnp.PingPong(c1.Bootstrap(ctx))
+				assert.NoError(t, bs.Resolve(ctx))
 				r.Fulfill(bs)
 			}()
 		}

So the problem I think arises when a promise resolves to another promise (in this case a question; not sure if that matters).

I'm attaching a log for the ordering problem. Need to dig in still.
log.txt

[zenhack]

Solved the deadlock (kinda... see the commit message), still need to work out the ordering problem.

[lthibault]

@zenhack Do we have an issue open for the shutdown bug?

[zenhack]

Not that I know of; I have no memory of having seen something like this before.

[zenhack]

I merged back in main, and also reworked the way NewLocalPromise works to avoid some dodgy code that I think has race conditions. I held out some hope that this would fix the ordering bug, but it seems to have no effect.

[zenhack]

Discussed some on matrix, but for posterity: it's not clear to me that the tribble 4-way race condition actually requires three distinct vats, so it could be a problem even in a level 1 implementation, if each link in the promise chain crosses the network. The failure we're seeing matches this description, so I am wondering if it is a symptom of the 4-way race condition. The flow looks something like:

• vat A requests vat B's bootstrap interface, and starts making calls on it (before receiving the return). The question for the bootstrap is the first promise.
• vat B returns a senderPromsie as its bootstrap.
• At some point later, vat B requests vat A's bootstrap interface and fulfills the promise it sent with the result. Note that the bug crops up regardless of whether or not vat B waits for the return for the bootstrap message. If it does not there are three promises, but even if it does there are still two.

I'm going to put this PR down until I've more deeply understood the implications. I still haven't totally grokked what exactly goes wrong in the 4-way race condition, only that there is an assumption being violated (i.e. I don't know what an example failure due to this issue should look like). I plan on figuring this out, and probably also stepping back to fix the way we do promise resolution in the library; regardless of whether this hunch is correct, we'll need to for 3PH.

[zenhack]

Per #494 (comment), my suspicion was correct: 3PH is not required for the race, and it seems like a good bed that's what's causing the failure. So I'll work on that refactor before coming back to this.

[zenhack]

I've merged in main and #520, still reconciling the latter.

[zenhack]

Superseded by #530, closing.