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

WIP: Handle incoming resolve messages, take 2 #530

Merged
merged 16 commits into from
Jun 29, 2023

Conversation

zenhack
Copy link
Contributor

@zenhack zenhack commented Jun 17, 2023

supersedes #480, whose conflicts were formidable enough that I had to cherry-pick and transcribe somewhat manually. It is conceptually the same change.

Some of the tests are failing, so this is a draft for now.

Still TODO:

- We need to handle disembargos with target = (importedCap = ...).
- Testing.
The := below is sufficient, since this isn't actually used before that.
Also, along the way, use the snapshot from resultsCapTable
instead of calling .Snapshot() on the client.
The test is currently failing; we're getting back an abort message
complaining that the export entry is not a promise -- need to
investigate.

Aside from that failure, this caught a bug: we need to check if
ClientState.Metadata is nil, which is possible if the client itself is
null, and seems to happen to the promise after it is resolved.

TODO: we should de-dup the logic between releaseExport and
releaseExports. This is not entirely trivial though, because the
latter is executed after we've wiped the exports table.
@zenhack zenhack marked this pull request as draft June 17, 2023 05:53
...also failing (hanging) for now.
@zenhack
Copy link
Contributor Author

zenhack commented Jun 17, 2023

(Note: there are some bits from the other PR that I haven't pulled in yet, which may address some of the test failures)

...which we were forgetting to set. We can query snapshot for this
anyway, so just get rid of it -- single source of truth and all.
When looking at the target of the promise, we want to look at its
*resolution.*
Otherwise we get a runtime error sometimes.
...the latter often just results in me staring at cascading errors,
which is unhelpful.
Push the logic for flushing the answerqueue into the Promise type
itself. This is much cleaner, and avoids some racy logic that I'm not
sure was correct.
@zenhack zenhack requested a review from lthibault June 26, 2023 17:57
@zenhack zenhack marked this pull request as ready for review June 29, 2023 21:00
@zenhack zenhack merged commit 7e1bedd into capnproto:main Jun 29, 2023
0 of 4 checks passed
@zenhack zenhack deleted the handle-resolve-take-2 branch June 29, 2023 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants