Skip to content

Only allow 'async' ABI options for 'async'-typed function imports/exports#646

Open
lukewagner wants to merge 2 commits intomainfrom
restrict-sync
Open

Only allow 'async' ABI options for 'async'-typed function imports/exports#646
lukewagner wants to merge 2 commits intomainfrom
restrict-sync

Conversation

@lukewagner
Copy link
Copy Markdown
Member

There's currently a bug in the spec whereby if you lift a sync-(i.e., non-async-)typed function with the async callback ABI option, the spec rules still try to acquire and release the per-component-instance exclusive lock (which is meant to only allow one turn of the event loop to run at a time), even though this might block and sync functions are not allowed to block before returning a value. It's also not clear what the "right" fix here should be: the goal of async callback is to prevent mid-event-loop-turn reentrance so that a single global linear memory shadow-stack can be used and the obvious spec-fixes would break that. Instead, it seems simpler to rule out a whole row of the testing matrix and reject at validation time the use of async ABI options when lifting or lowering non-async-typed functions. This could be viewed as a trailing simplification allowed by the switch of async from hint to effect.

For the use case of "doing some blocking I/O after returning a value from a synchronous function", cooperative threads provide a simpler solution, since cooperative threads already come with the requirement that cooperative threads each need their own linear-memory shadow-stack due to non-LIFO execution interleaving.

The theory is that this isn't being used (since all the new Preview 3 WASI worlds have async exports) and so shouldn't cause churn for forthcoming Preview 3 release, but I'll leave the PR open to collect feedback for a while.

Copy link
Copy Markdown
Collaborator

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

At a high-level I'm fine with this and it sounds good. To double-check the feasibility of this change I've made this change in wasm-tools and then updated Wasmtime as well. I've then verified that the final binary runs all wit-bindgen tests as-is with no modifications necessary.

While this is an impactful change that required a number of updates to tests throughout the projects, I don't feel that anything is a blocker. Effectively all of these tests needed to be updated anyway once async was added to function types and they otherwise never had a forcing function to enact the change. Now that the forcing function's here all the changes seem reasonable.

For this repository itself I also needed to apply this diff:

Details
diff --git a/test/async/cross-abi-calls.wast b/test/async/cross-abi-calls.wast
index b631f61..8f1370b 100644
--- a/test/async/cross-abi-calls.wast
+++ b/test/async/cross-abi-calls.wast
@@ -175,62 +175,67 @@
       (export "task.return16" (func $task.return16))
       (export "task.return17" (func $task.return17))
     ))))
-    (func (export "sync-4-param") (param "a" u32) (param "b" u64) (param "c" f32) (param "d" f64)
+    (func (export "sync-4-param") async (param "a" u32) (param "b" u64) (param "c" f32) (param "d" f64)
       (canon lift (core func $core "sync-4-param"))
     )
-    (func (export "sync-5-param") (param "a" u32) (param "b" u64) (param "c" f32) (param "d" f64) (param "e" u32)
+    (func (export "sync-5-param") async (param "a" u32) (param "b" u64) (param "c" f32) (param "d" f64) (param "e" u32)
       (canon lift (core func $core "sync-5-param"))
     )
-    (func (export "sync-17-param") (param "a" u32) (param "b" u64) (param "c" f32) (param "d" f64)
+    (func (export "sync-17-param") async
+                                   (param "a" u32) (param "b" u64) (param "c" f32) (param "d" f64)
                                    (param "e" u32) (param "f" u64) (param "g" f32) (param "h" f64)
                                    (param "i" u32) (param "j" u64) (param "k" f32) (param "l" f64)
                                    (param "m" u32) (param "n" u64) (param "o" f32) (param "p" f64)
                                    (param "q" u32)
       (canon lift (core func $core "sync-17-param") (memory $memory "mem") (realloc (func $memory "realloc")))
     )
-    (func (export "sync-1-result") (result f64)
+    (func (export "sync-1-result") async (result f64)
       (canon lift (core func $core "sync-1-result"))
     )
-    (func (export "sync-16-result") (result (tuple u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64))
+    (func (export "sync-16-result") async (result (tuple u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64))
       (canon lift (core func $core "sync-16-result") (memory $memory "mem"))
     )
-    (func (export "sync-17-result") (result (tuple u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64 u32))
+    (func (export "sync-17-result") async (result (tuple u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64 u32))
       (canon lift (core func $core "sync-17-result") (memory $memory "mem"))
     )
-    (func (export "async-4-param") (param "a" u32) (param "b" u64) (param "c" f32) (param "d" f64)
+    (func (export "async-4-param") async
+                                   (param "a" u32) (param "b" u64) (param "c" f32) (param "d" f64)
       (canon lift (core func $core "async-4-param") async (callback (func $core "unreachable-cb")))
     )
-    (func (export "async-5-param") (param "a" u32) (param "b" u64) (param "c" f32) (param "d" f64) (param "e" u32)
+    (func (export "async-5-param") async
+                                   (param "a" u32) (param "b" u64) (param "c" f32) (param "d" f64) (param "e" u32)
       (canon lift (core func $core "async-5-param") async (callback (func $core "unreachable-cb")))
     )
-    (func (export "async-17-param") (param "a" u32) (param "b" u64) (param "c" f32) (param "d" f64)
+    (func (export "async-17-param") async
+                                    (param "a" u32) (param "b" u64) (param "c" f32) (param "d" f64)
                                     (param "e" u32) (param "f" u64) (param "g" f32) (param "h" f64)
                                     (param "i" u32) (param "j" u64) (param "k" f32) (param "l" f64)
                                     (param "m" u32) (param "n" u64) (param "o" f32) (param "p" f64)
                                     (param "q" u32)
       (canon lift (core func $core "async-17-param") async (callback (func $core "unreachable-cb")) (memory $memory "mem") (realloc (func $memory "realloc")))
     )
-    (func (export "async-1-result") (result f64)
+    (func (export "async-1-result") async (result f64)
       (canon lift (core func $core "async-1-result") async (callback (func $core "unreachable-cb")))
     )
-    (func (export "async-16-result") (result (tuple u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64))
+    (func (export "async-16-result") async (result (tuple u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64))
       (canon lift (core func $core "async-16-result") async (callback (func $core "unreachable-cb")))
     )
-    (func (export "async-17-result") (result (tuple u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64 u32))
+    (func (export "async-17-result") async (result (tuple u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64 u32))
       (canon lift (core func $core "async-17-result") async (callback (func $core "unreachable-cb")) (memory $memory "mem") (realloc (func $memory "realloc")))
     )
   )
   (component $Bottom
-    (import "func-4-param" (func $func-4-param (param "a" u32) (param "b" u64) (param "c" f32) (param "d" f64)))
-    (import "func-5-param" (func $func-5-param (param "a" u32) (param "b" u64) (param "c" f32) (param "d" f64) (param "e" u32)))
-    (import "func-17-param" (func $func-17-param (param "a" u32) (param "b" u64) (param "c" f32) (param "d" f64)
+    (import "func-4-param" (func $func-4-param async (param "a" u32) (param "b" u64) (param "c" f32) (param "d" f64)))
+    (import "func-5-param" (func $func-5-param async (param "a" u32) (param "b" u64) (param "c" f32) (param "d" f64) (param "e" u32)))
+    (import "func-17-param" (func $func-17-param async
+                                                 (param "a" u32) (param "b" u64) (param "c" f32) (param "d" f64)
                                                  (param "e" u32) (param "f" u64) (param "g" f32) (param "h" f64)
                                                  (param "i" u32) (param "j" u64) (param "k" f32) (param "l" f64)
                                                  (param "m" u32) (param "n" u64) (param "o" f32) (param "p" f64)
                                                  (param "q" u32)))
-    (import "func-1-result" (func $func-1-result (result f64)))
-    (import "func-16-result" (func $func-16-result (result (tuple u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64))))
-    (import "func-17-result" (func $func-17-result (result (tuple u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64 u32))))
+    (import "func-1-result" (func $func-1-result async (result f64)))
+    (import "func-16-result" (func $func-16-result async (result (tuple u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64))))
+    (import "func-17-result" (func $func-17-result async (result (tuple u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64 u32 u64 f32 f64 u32))))
     (core module $Memory (memory (export "mem") 1))
     (core instance $memory (instantiate $Memory))
     (core module $Core
@@ -408,18 +413,18 @@
       (export "sync-17-result" (func $sync-17-result))
       (export "async-17-result" (func $async-17-result))
     ))))
-    (func (export "call-sync-4-param") (result u32) (canon lift (core func $core "call-sync-4-param")))
-    (func (export "call-async-4-param") (result u32) (canon lift (core func $core "call-async-4-param")))
-    (func (export "call-sync-5-param") (result u32) (canon lift (core func $core "call-sync-5-param")))
-    (func (export "call-async-5-param") (result u32) (canon lift (core func $core "call-async-5-param")))
-    (func (export "call-sync-17-param") (result u32) (canon lift (core func $core "call-sync-17-param")))
-    (func (export "call-async-17-param") (result u32) (canon lift (core func $core "call-async-17-param")))
-    (func (export "call-sync-1-result") (result u32) (canon lift (core func $core "call-sync-1-result")))
-    (func (export "call-async-1-result") (result u32) (canon lift (core func $core "call-async-1-result")))
-    (func (export "call-sync-16-result") (result u32) (canon lift (core func $core "call-sync-16-result")))
-    (func (export "call-async-16-result") (result u32) (canon lift (core func $core "call-async-16-result")))
-    (func (export "call-sync-17-result") (result u32) (canon lift (core func $core "call-sync-17-result")))
-    (func (export "call-async-17-result") (result u32) (canon lift (core func $core "call-async-17-result")))
+    (func (export "call-sync-4-param") async (result u32) (canon lift (core func $core "call-sync-4-param")))
+    (func (export "call-async-4-param") async (result u32) (canon lift (core func $core "call-async-4-param")))
+    (func (export "call-sync-5-param") async (result u32) (canon lift (core func $core "call-sync-5-param")))
+    (func (export "call-async-5-param") async (result u32) (canon lift (core func $core "call-async-5-param")))
+    (func (export "call-sync-17-param") async (result u32) (canon lift (core func $core "call-sync-17-param")))
+    (func (export "call-async-17-param") async (result u32) (canon lift (core func $core "call-async-17-param")))
+    (func (export "call-sync-1-result") async (result u32) (canon lift (core func $core "call-sync-1-result")))
+    (func (export "call-async-1-result") async (result u32) (canon lift (core func $core "call-async-1-result")))
+    (func (export "call-sync-16-result") async (result u32) (canon lift (core func $core "call-sync-16-result")))
+    (func (export "call-async-16-result") async (result u32) (canon lift (core func $core "call-async-16-result")))
+    (func (export "call-sync-17-result") async (result u32) (canon lift (core func $core "call-sync-17-result")))
+    (func (export "call-async-17-result") async (result u32) (canon lift (core func $core "call-async-17-result")))
   )
   (instance $top (instantiate $Top))
   (instance $bottom-to-sync (instantiate $Bottom
diff --git a/test/async/trap-on-reenter.wast b/test/async/trap-on-reenter.wast
index 5ce72a0..f363f6d 100644
--- a/test/async/trap-on-reenter.wast
+++ b/test/async/trap-on-reenter.wast
@@ -12,13 +12,13 @@
     )
   )
   (core instance $core_inner (instantiate $CoreInner))
-  (func $a (canon lift
+  (func $a async (canon lift
     (core func $core_inner "a")
     async (callback (func $core_inner "a-cb"))
   ))

   (component $Child
-    (import "a" (func $a))
+    (import "a" (func $a async))

     (core module $Memory (memory (export "mem") 1))
     (core instance $memory (instantiate $Memory))
@@ -37,7 +37,7 @@
     (core instance $core_child (instantiate $CoreChild (with "" (instance
       (export "a" (func $a'))
     ))))
-    (func (export "b") (canon lift
+    (func (export "b") async (canon lift
       (core func $core_child "b")
       async (callback (func $core_child "b-cb"))
     ))
@@ -57,7 +57,7 @@
   (core instance $core_outer (instantiate $CoreOuter (with "" (instance
     (export "b" (func $b))
   ))))
-  (func $c (export "c") (canon lift
+  (func $c (export "c") async (canon lift
     (core func $core_outer "c")
     async (callback (func $core_outer "c-cb"))
   ))

to get the tests passing in Wasmtime

@lukewagner
Copy link
Copy Markdown
Member Author

Thanks! I added that diff to this PR.

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