Support linear handles in Oak Runtime. #1915

wildarch · 2021-03-19T10:10:37Z

This also migrates the hello_world example to use linear-handles.

Note: I was planning to migrate all of the examples first, but that is causing tests to fail, as some examples have a dev dependency on oak_runtime.

Tracking issue: #1854.

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.

rbehjati · 2021-03-19T15:07:00Z

oak_runtime/src/lib.rs

@@ -394,10 +394,14 @@ impl AuxServer {
    ) -> Self {
        let (termination_notificiation_sender, termination_notificiation_receiver) =
            tokio::sync::oneshot::channel::<()>();
+        let runtime_proxy = runtime.clone().proxy_for_new_node(name);


Is this actually a new node? Or is it sufficient to call runtime.clone()?

The AuxServer is definitely on its own thread, and it communicates with other nodes using handles, so I would assume it qualifies as a node?

The reason I am creating a proxy here is that the thread local variable stores a proxy, not an Arc<Runtime>

Are you sure it communicates with other nodes using handles? I think it should not. Also, it does not implement Node.
Do any of the tests fail if you remove proxy_for_new_node(name)?

This might also explain why node 1 does not exist anymore (in abitest). We have two AuxServers, one for metrics and one for introspection. I think these servers are being created before any other node. I am guessing with the call to proxy_for_new_node here, these servers consume the first two node_ids, without actually being nodes.

Aah, sorry you are right, I think the AuxServer does not need to set this.

I will remove it, thanks 😄

rbehjati · 2021-03-20T00:44:52Z

oak_runtime/src/proxy.rs

+    }
+}
+
+std::thread_local! {


I understand that this is needed to implement handle_clone and channel_close. But I wonder if there is a better way to do this. What I don't like about this approach is that the call to set_as_current seems to be at rather random places. Basically whenever a new thread is spawned. Could spawning a thread and setting RUNTIME_PROXY be encapsulated into a function?

You're right that we require a call to set_as_current in every thread, but I am not sure if there is a way to make this cleaner. Threads are created in different ways, sometimes we need to call it in tokio worker threads, sometimes in a plain thread, so I don't think we can create one function that fits all use cases (and we would still have to get everyone starting a new thread to use it).

Yeah. Unfortunately that is what I thought too, but I was hoping you might have a solution :)

This also migrates the hello_world example to use linear-handles.

I am not sure why node 1 does not exist anymore during the introspection tests, but any other node should do fine too.

rbehjati · 2021-03-25T16:49:19Z

examples/abitest/client/cpp/abitest.cc

@@ -195,25 +195,25 @@ bool run_aux_tests() {
  success &= check_page(&introspect, "/graph");
  success &= check_page(&introspect, "/objcount");
  success &= check_page(&introspect, "/node/0");
-  success &= check_page(&introspect, "/node/1");
+  success &= check_page(&introspect, "/node/3");


Is this change still needed? Or does it now work with "/node/1".

Aah, looks like it works with 1 now. I'm not sure what changed, do you have any ideas?

I think that call to proxy_for_new_node in AuxServer was unnecessarily increasing the node_id.

rbehjati

LGTM if the tests pass :)

rbehjati · 2021-03-29T18:28:25Z

examples/aggregator/client/android/cpp/client.cc

@@ -55,7 +55,7 @@ JNIEXPORT void JNICALL Java_com_google_oak_aggregator_MainActivity_createChannel
  // The particular value corresponds to the hash on the `aggregator.wasm` line in
  // https://github.com/project-oak/oak/blob/hashes/reproducibility_index.
  oak::label::Label label = oak::WebAssemblyModuleHashLabel(
-      absl::HexStringToBytes("a3d55c1e3af8dc34dfe5a3736bfcbd7db8541a607a33089730e81eda3d5b63bc"));
+      absl::HexStringToBytes("4b652112bb9904976fd5a215a4df081956165f1dbc19bf6c77e0eef05c4bc256"));


Out of curiosity: do you remember why you had to make this change (or the other change that is reverted in this commit), and where the other hash came from? We had a strange discussion earlier involving exactly these two hash values: https://project-oak.slack.com/archives/CHE9E13C3/p1615996966001400?thread_ts=1615996957.001300&cid=CHE9E13C3

github-actions · 2021-03-29T18:46:19Z

Reproducibility Index:

c83ed9619f0b05944909a7e006968b547fe487f1e8892ba40ebc1a525893a2dc  ./examples/abitest/bin/abitest_0_frontend.wasm
247471b75dbef95ff35065fdc69b0ff1ea147a6c1d00c8145d852262fe152abb  ./examples/abitest/bin/abitest_1_backend.wasm
3259f401209d5f874323c5adbb073bb879bd5c7a057e2bdb0d03573cf90c7f5a  ./examples/abitest/bin/abitest_linear_handles.wasm
4b652112bb9904976fd5a215a4df081956165f1dbc19bf6c77e0eef05c4bc256  ./examples/aggregator/bin/aggregator.wasm
511f34560a7a49125da2a8295c00cf5988663a5c26859d8f888d6459ed016064  ./examples/chat/bin/chat.wasm
7c8137088571877f7c1485bc0466fb3c3c641a92833dc53525d5210323235ebb  ./examples/hello_world/bin/hello_world.wasm
cd27f328c445dff71a523dd96e0580217f96b1566d5151424de7de02639af937  ./examples/hello_world/bin/translator.wasm
c4b39c77771d3c2a8373233621506328ff147653885b5fad6321cbe9bcdf5340  ./examples/http_server/bin/http_server.wasm
5afd3f97518c7accaf06114a9522976d96c75425fa2d427c7f2110162be523fa  ./examples/injection/bin/injection.wasm
209b7403a1ca6c68375450e9df656a16b6da6f18998f3503edaa1c13c5ccb116  ./examples/private_set_intersection/bin/private_set_intersection.wasm
7421c7cad50da1baa1c5ddbd3d2b3fe3a8317dfb53b3431e98ef2cff5b7f3c6a  ./examples/private_set_intersection/bin/private_set_intersection_handler.wasm
3b9d9d6ff34b9e6f454af93a3c12d17ca71afecee349a094b74bfc60890f0dc3  ./examples/proxy_attestation/bin/proxy_attestation_example.wasm
cd27f328c445dff71a523dd96e0580217f96b1566d5151424de7de02639af937  ./examples/translator/bin/translator.wasm
db1d07eb17d9de5e84a0f73a17734e35995a40ded5fd505b95dc8aa1bcfbc19a  ./examples/trusted_database/bin/trusted_database.wasm
c041fb1dd011872cc1f083304a5335a41ebd153fd99bfc5d982ce9ad2c1cd2c9  ./oak_loader/bin/oak_loader

Reproducibility Index diff:

diff --git a/reproducibility_index b/reproducibility_index
index a6e5c3c..5265614 100644
--- a/reproducibility_index
+++ b/reproducibility_index
@@ -3,7 +3,7 @@ c83ed9619f0b05944909a7e006968b547fe487f1e8892ba40ebc1a525893a2dc  ./examples/abi
 3259f401209d5f874323c5adbb073bb879bd5c7a057e2bdb0d03573cf90c7f5a  ./examples/abitest/bin/abitest_linear_handles.wasm
 4b652112bb9904976fd5a215a4df081956165f1dbc19bf6c77e0eef05c4bc256  ./examples/aggregator/bin/aggregator.wasm
 511f34560a7a49125da2a8295c00cf5988663a5c26859d8f888d6459ed016064  ./examples/chat/bin/chat.wasm
-e88b326c06ea2b21a953d7861a17941aef646398d336fe47b769894bb742d9d1  ./examples/hello_world/bin/hello_world.wasm
+7c8137088571877f7c1485bc0466fb3c3c641a92833dc53525d5210323235ebb  ./examples/hello_world/bin/hello_world.wasm
 cd27f328c445dff71a523dd96e0580217f96b1566d5151424de7de02639af937  ./examples/hello_world/bin/translator.wasm
 c4b39c77771d3c2a8373233621506328ff147653885b5fad6321cbe9bcdf5340  ./examples/http_server/bin/http_server.wasm
 5afd3f97518c7accaf06114a9522976d96c75425fa2d427c7f2110162be523fa  ./examples/injection/bin/injection.wasm
@@ -12,4 +12,4 @@ c4b39c77771d3c2a8373233621506328ff147653885b5fad6321cbe9bcdf5340  ./examples/htt
 3b9d9d6ff34b9e6f454af93a3c12d17ca71afecee349a094b74bfc60890f0dc3  ./examples/proxy_attestation/bin/proxy_attestation_example.wasm
 cd27f328c445dff71a523dd96e0580217f96b1566d5151424de7de02639af937  ./examples/translator/bin/translator.wasm
 db1d07eb17d9de5e84a0f73a17734e35995a40ded5fd505b95dc8aa1bcfbc19a  ./examples/trusted_database/bin/trusted_database.wasm
-2ca6b191518ea6c966b5102c669358f916fbe2f2156a227576e92f5914ea9169  ./oak_loader/bin/oak_loader
+c041fb1dd011872cc1f083304a5335a41ebd153fd99bfc5d982ce9ad2c1cd2c9  ./oak_loader/bin/oak_loader

google-cla bot added the cla: yes label Mar 19, 2021

wildarch force-pushed the oak_runtime_linear_handles branch 2 times, most recently from 67e9918 to 5ccc85a Compare March 19, 2021 11:00

wildarch marked this pull request as ready for review March 19, 2021 11:02

wildarch requested a review from rbehjati March 19, 2021 11:02

wildarch mentioned this pull request Mar 19, 2021

Migrate examples use linear handles (except abitest) #1909

Merged

6 tasks

rbehjati reviewed Mar 22, 2021

View reviewed changes

wildarch force-pushed the oak_runtime_linear_handles branch from 3a1b8e7 to 9f9d7bb Compare March 25, 2021 13:00

wildarch requested a review from rbehjati March 25, 2021 13:05

wildarch mentioned this pull request Mar 25, 2021

Migrate Abitest to linear handles #1916

Closed

6 tasks

wildarch and others added 4 commits March 25, 2021 16:11

Support linear handles in Oak Runtime.

7749ef3

This also migrates the hello_world example to use linear-handles.

Fix abitest tests.

9f4e43e

I am not sure why node 1 does not exist anymore during the introspection tests, but any other node should do fine too.

Update aggregator and private_set_intersection hashes.

4133208

Don't create a proxy for AuxServer.

22d4726

wildarch force-pushed the oak_runtime_linear_handles branch from dd7c7c0 to 22d4726 Compare March 25, 2021 16:27

rbehjati reviewed Mar 25, 2021

View reviewed changes

Switch back to node/1 and update psi signature.

87f54c7

rbehjati approved these changes Mar 25, 2021

View reviewed changes

wildarch added 2 commits March 29, 2021 07:48

Fix broken doc links.

9321dec

Update aggregator hash.

755d82d

wildarch merged commit 1d38b21 into project-oak:main Mar 29, 2021

wildarch deleted the oak_runtime_linear_handles branch March 29, 2021 18:14

rbehjati reviewed Mar 29, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support linear handles in Oak Runtime. #1915

Support linear handles in Oak Runtime. #1915

wildarch commented Mar 19, 2021 •

edited

Loading

rbehjati Mar 19, 2021

wildarch Mar 25, 2021

rbehjati Mar 25, 2021

rbehjati Mar 25, 2021

wildarch Mar 25, 2021

rbehjati Mar 25, 2021

rbehjati Mar 20, 2021

wildarch Mar 25, 2021

rbehjati Mar 25, 2021

rbehjati Mar 25, 2021

wildarch Mar 25, 2021

rbehjati Mar 25, 2021

rbehjati left a comment

rbehjati Mar 29, 2021

github-actions bot commented Mar 29, 2021

Support linear handles in Oak Runtime. #1915

Support linear handles in Oak Runtime. #1915

Conversation

wildarch commented Mar 19, 2021 • edited Loading

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rbehjati left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 29, 2021

wildarch commented Mar 19, 2021 •

edited

Loading