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

Allow passing borrowed handles to SDK functions. #1843

Merged
merged 2 commits into from
Feb 12, 2021

Conversation

wildarch
Copy link
Contributor

This is currently not needed because handles implement Copy, but once
they are made linear, using borrowed handles will be the only way to use
these functions without giving up ownership of the handle.

Checklist

  • Pull request affects core Oak functionality (e.g. runtime, SDK, ABI)
    • I have written tests that cover the code changes.
    • I have checked that these tests are run by
      Cloudbuild
    • I have updated documentation accordingly.
    • I have raised an issue to
      cover any TODOs and/or unfinished work.
  • Pull request includes prototype/experimental work that is under
    construction.

Related issue: #1686

@google-cla google-cla bot added the cla: yes label Jan 22, 2021
@wildarch wildarch marked this pull request as ready for review January 22, 2021 12:02
@wildarch
Copy link
Contributor Author

Failed test looks like a flake:

Get https://registry-1.docker.io/v2/debian/snapshot/manifests/buster-20201012: received unexpected HTTP status: 502 Bad Gateway

@@ -67,11 +68,11 @@ pub mod proto {

// Build a chunk of memory that is suitable for passing to oak_abi::wait_on_channels,
// holding the given collection of channel handles.
fn new_handle_space(handles: &[ReadHandle]) -> Vec<u8> {
fn new_handle_space<R: Borrow<ReadHandle>>(handles: &[R]) -> Vec<u8> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this just temporary until we modify all the call places to use actual borrows, or do you think we will want to keep it after that? I think it does not look very nice, so I hope we can get rid of it at some point (perhaps worth a TODO + issue?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once linear handles are fully implemented and the default, then this can be simplified again. I'll add a TODO and link to the linear handles issue.

This is currently not needed because handles implement `Copy`, but once
they are made linear, using borrowed handles will be the only way to use
these functions without giving up ownership of the handle.
@wildarch wildarch merged commit cf88ea8 into project-oak:main Feb 12, 2021
@wildarch wildarch deleted the borrow_handles_sdk branch February 12, 2021 16:29
@github-actions
Copy link

Reproducibility Index:

f7e161a3d8e803b44feaa5b1daf8588a69f3fce989f16924bb1de371b99c5848  ./examples/abitest/bin/abitest_0_frontend.wasm
bc8a0d13bbd73007aa0fad6781b324c5aefc9caf840d639b38a232f0f6d4c9f5  ./examples/abitest/bin/abitest_1_backend.wasm
a5b271718d45f17bad093efc9dff2749d7f26ceadfd81120be6c20b169bc6dfb  ./examples/abitest/bin/abitest_linear_handles.wasm
f097be294bbabde65ef042e924a6f6698b2f6bd0b512c0feb04e6acd97fc28f4  ./examples/aggregator/bin/aggregator.wasm
6694e44c1253b3282fa0e44bf0de7416dab1317f2184e3143ce2966259ff92ec  ./examples/chat/bin/chat.wasm
fa21fe72231e9434cb87f5f9673712bb53f3ec5001038a13c794c02b1d6e3f9f  ./examples/hello_world/bin/hello_world.wasm
e7ad7909067cfde30d316167c61e7394ccbad87b6169e95155590d48c4741eb9  ./examples/hello_world/bin/translator.wasm
602ade553c53bcdc913517683efb1a114997f8030d15152dc9b245e9f3ecd751  ./examples/http_server/bin/http_server.wasm
38f93f321bc1135b176dd6ee3a2fb4088187212c59307fb93e25dff4eefbeaa5  ./examples/injection/bin/injection.wasm
d0c551970a98c5df8c967d7fc0e8b9fdc37f29aaf0fbdf569712bff7b43540a8  ./examples/private_set_intersection/bin/private_set_intersection.wasm
7ad15d9ca0e7d1444068deb8ec98f13fa25a9433844315e9bd936edb8e924b69  ./examples/private_set_intersection/bin/private_set_intersection_handler.wasm
a78a92c27d4d7bfe957ead09797b915409dea32791e95a0096f4c0d93542e55f  ./examples/proxy_attestation/bin/proxy_attestation_example.wasm
e7ad7909067cfde30d316167c61e7394ccbad87b6169e95155590d48c4741eb9  ./examples/translator/bin/translator.wasm
445d94706ead306996a18b7a078bf2b44948759fcdabdf89369785f8d8400fa0  ./examples/trusted_database/bin/trusted_database.wasm
ab3bc68ef7f8ca524279c826a02a8c84355fcc141ee283c624098d375961857e  ./oak_loader/bin/oak_loader

Reproducibility Index diff:

diff --git a/reproducibility_index b/reproducibility_index
index 6700c1a..9b93629 100644
--- a/reproducibility_index
+++ b/reproducibility_index
@@ -1,15 +1,15 @@
-3b9bf47c09f52c2035409b29aa7ccf3dbf0b198d542c9142ee0803e2780255db  ./examples/abitest/bin/abitest_0_frontend.wasm
-20f395e082b2dd75b698bd2c1b7542a11fae757c229de4129f268e6b8b2abe53  ./examples/abitest/bin/abitest_1_backend.wasm
+f7e161a3d8e803b44feaa5b1daf8588a69f3fce989f16924bb1de371b99c5848  ./examples/abitest/bin/abitest_0_frontend.wasm
+bc8a0d13bbd73007aa0fad6781b324c5aefc9caf840d639b38a232f0f6d4c9f5  ./examples/abitest/bin/abitest_1_backend.wasm
 a5b271718d45f17bad093efc9dff2749d7f26ceadfd81120be6c20b169bc6dfb  ./examples/abitest/bin/abitest_linear_handles.wasm
-8d890bcade31c6c8ab1926f5ece344de1664321b96401d7f3885ebafad570a68  ./examples/aggregator/bin/aggregator.wasm
-77e6001774063f7e351d5ec13259cf07301b8a8ba79e9a11496ba8c651892ce3  ./examples/chat/bin/chat.wasm
-9ad7c6cb5e17451fcdff397564a5ae963dfcf4863c204b6edba282e0dc10bd7a  ./examples/hello_world/bin/hello_world.wasm
-2235d306f8c3baf5d5e5736f43972d6490f52570ad87b5ab1422c5ce079b9537  ./examples/hello_world/bin/translator.wasm
-321d2ef8f44c6a5d8ea41462098cc69ce6f1ccf8a24072900cb8f02381206d50  ./examples/http_server/bin/http_server.wasm
-84076e05bf95ecd97dc29079b5c7f0690b3ba52838be0504f0642484c86c8784  ./examples/injection/bin/injection.wasm
-87a30147d0f0c4e194ffb9d3b74ce083084e3deb505cd7a99b183b202843934a  ./examples/private_set_intersection/bin/private_set_intersection.wasm
-dbde9e581d72c9f4307d5dbd4504a3ede20b7bd7359fe389699fb9712ac8baf3  ./examples/private_set_intersection/bin/private_set_intersection_handler.wasm
-409182b9bd6aa0701f0bc06ecadd9fa2dd8d1c0cd2de45a0e250c888e4cd0bbf  ./examples/proxy_attestation/bin/proxy_attestation_example.wasm
-2235d306f8c3baf5d5e5736f43972d6490f52570ad87b5ab1422c5ce079b9537  ./examples/translator/bin/translator.wasm
-8999999ce4051b31d0bcd7a869b656883c8828663f44378f30f7dfe81da8f681  ./examples/trusted_database/bin/trusted_database.wasm
-c6172f7b5a682d36db61daa546e8ae1a1233fb6d0785e91c6730026b132ebaed  ./oak_loader/bin/oak_loader
+f097be294bbabde65ef042e924a6f6698b2f6bd0b512c0feb04e6acd97fc28f4  ./examples/aggregator/bin/aggregator.wasm
+6694e44c1253b3282fa0e44bf0de7416dab1317f2184e3143ce2966259ff92ec  ./examples/chat/bin/chat.wasm
+fa21fe72231e9434cb87f5f9673712bb53f3ec5001038a13c794c02b1d6e3f9f  ./examples/hello_world/bin/hello_world.wasm
+e7ad7909067cfde30d316167c61e7394ccbad87b6169e95155590d48c4741eb9  ./examples/hello_world/bin/translator.wasm
+602ade553c53bcdc913517683efb1a114997f8030d15152dc9b245e9f3ecd751  ./examples/http_server/bin/http_server.wasm
+38f93f321bc1135b176dd6ee3a2fb4088187212c59307fb93e25dff4eefbeaa5  ./examples/injection/bin/injection.wasm
+d0c551970a98c5df8c967d7fc0e8b9fdc37f29aaf0fbdf569712bff7b43540a8  ./examples/private_set_intersection/bin/private_set_intersection.wasm
+7ad15d9ca0e7d1444068deb8ec98f13fa25a9433844315e9bd936edb8e924b69  ./examples/private_set_intersection/bin/private_set_intersection_handler.wasm
+a78a92c27d4d7bfe957ead09797b915409dea32791e95a0096f4c0d93542e55f  ./examples/proxy_attestation/bin/proxy_attestation_example.wasm
+e7ad7909067cfde30d316167c61e7394ccbad87b6169e95155590d48c4741eb9  ./examples/translator/bin/translator.wasm
+445d94706ead306996a18b7a078bf2b44948759fcdabdf89369785f8d8400fa0  ./examples/trusted_database/bin/trusted_database.wasm
+ab3bc68ef7f8ca524279c826a02a8c84355fcc141ee283c624098d375961857e  ./oak_loader/bin/oak_loader

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants