Repository: denoland/deno_core (rust)

Issue:
`op_close` with `WasmStreamingResource` invokes JS from an op while `OpState` is borrowed
I get a panic with the following code

```js
const makeEnv = (...envs) => {
  return new Proxy(envs, {
    get(_target, prop, _receiver) {
      console.log("get", prop); // remove this and it won't panic
      for (const env of envs) {
        if (Object.hasOwnProperty.call(env, prop)) {
          return env[prop];
        }
      }
      return (...args) => {
        console.error(`NOT IMPLEMENTED: ${prop}`, args);
      };
    },
  });
};

const { instance } = await WebAssembly.instantiateStreaming(
  fetch(new URL("./static/main.wasm", import.meta.url)),
  {
    env: makeEnv({
      printf: function () {
        console.log(arguments);
      },
    }),
  },
);

instance.exports.output(0);
```

```
============================================================
Deno has panicked. This is a bug in Deno. Please report this
at https://github.com/denoland/deno/issues/new.
If you can reliably reproduce this panic, include the
reproduction steps and re-run with the RUST_BACKTRACE=1 env
var set and include the backtrace in your report.

Platform: macos x86_64
Version: 1.36.4
Args: ["deno", "run", "-A", "script.js"]

thread 'main' panicked at 'already mutably borrowed: BorrowError', ext/node/global.rs:273:27
stack backtrace:
   0: _rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::result::unwrap_failed
   3: deno_node::global::current_mode
   4: deno_node::global::getter
   5: <extern "C" fn(A0,A1) .> R as v8::support::CFnFrom<F>>::mapping::c_fn
   6: __ZN2v88internal12_GLOBAL__N_134GetPropertyWithInterceptorInternalEPNS0_14LookupIteratorENS0_6HandleINS0_15InterceptorInfoEEEPb
   7: __ZN2v88internal6Object11GetPropertyEPNS0_14LookupIteratorEb
   8: __ZN2v88internal6LoadIC4LoadENS0_6HandleINS0_6ObjectEEENS2_INS0_4NameEEEbS4_
   9: __ZN2v88internal29Runtime_LoadNoFeedbackIC_MissEiPmPNS0_7IsolateE
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
```

Pull request:
fix: don't eval JS while OpState is held
Fixes #178

Patch:
diff --git a/core/ops_builtin.rs b/core/ops_builtin.rs
index 130f315..58c6118 100644
--- a/core/ops_builtin.rs
+++ b/core/ops_builtin.rs
@@ -129,18 +129,21 @@ pub async fn op_void_async_deferred() {}
 /// Remove a resource from the resource table.
 #[op2(core, fast)]
 pub fn op_close(
-  state: &mut OpState,
+  state: Rc<RefCell<OpState>>,
   #[smi] rid: ResourceId,
 ) -> Result<(), Error> {
-  state.resource_table.close(rid)?;
+  let resource = state.borrow_mut().resource_table.take_any(rid)?;
+  resource.close();
   Ok(())
 }
 
 /// Try to remove a resource from the resource table. If there is no resource
 /// with the specified `rid`, this is a no-op.
 #[op2(core, fast)]
-pub fn op_try_close(state: &mut OpState, #[smi] rid: ResourceId) {
-  let _ = state.resource_table.close(rid);
+pub fn op_try_close(state: Rc<RefCell<OpState>>, #[smi] rid: ResourceId) {
+  if let Ok(resource) = state.borrow_mut().resource_table.take_any(rid) {
+    resource.close();
+  }
 }
 
 #[op2(core)]
@@ -182,12 +185,14 @@ impl Resource for WasmStreamingResource {
 /// Feed bytes to WasmStreamingResource.
 #[op2(core, fast)]
 pub fn op_wasm_streaming_feed(
-  state: &mut OpState,
+  state: Rc<RefCell<OpState>>,
   #[smi] rid: ResourceId,
   #[buffer] bytes: &[u8],
 ) -> Result<(), Error> {
-  let wasm_streaming =
-    state.resource_table.get::<WasmStreamingResource>(rid)?;
+  let wasm_streaming = state
+    .borrow_mut()
+    .resource_table
+    .get::<WasmStreamingResource>(rid)?;
 
   wasm_streaming.0.borrow_mut().on_bytes_received(bytes);
 
@@ -268,21 +273,21 @@ async fn op_write(
 
 #[op2(core, fast)]
 fn op_read_sync(
-  state: &mut OpState,
+  state: Rc<RefCell<OpState>>,
   #[smi] rid: ResourceId,
   #[buffer] data: &mut [u8],
 ) -> Result<u32, Error> {
-  let resource = state.resource_table.get_any(rid)?;
+  let resource = state.borrow_mut().resource_table.get_any(rid)?;
   resource.read_byob_sync(data).map(|n| n as u32)
 }
 
 #[op2(core, fast)]
 fn op_write_sync(
-  state: &mut OpState,
+  state: Rc<RefCell<OpState>>,
   #[smi] rid: ResourceId,
   #[buffer] data: &[u8],
 ) -> Result<u32, Error> {
-  let resource = state.resource_table.get_any(rid)?;
+  let resource = state.borrow_mut().resource_table.get_any(rid)?;
   let nwritten = resource.write_sync(data)?;
   Ok(nwritten as u32)
 }


Test patch:
diff --git a/core/runtime/tests/misc.rs b/core/runtime/tests/misc.rs
index eedb0e9..2100311 100644
--- a/core/runtime/tests/misc.rs
+++ b/core/runtime/tests/misc.rs
@@ -321,6 +321,38 @@ fn terminate_execution() {
   terminator_thread.join().unwrap();
 }
 
+#[tokio::test]
+async fn wasm_streaming_op_invocation_in_import() {
+  let (mut runtime, _dispatch_count) = setup(Mode::Async);
+
+  // Run an infinite loop in WebAssembly code, which should be terminated.
+  runtime.execute_script_static("setup.js",
+                               r#"
+                                Deno.core.setWasmStreamingCallback((source, rid) => {
+                                  Deno.core.ops.op_wasm_streaming_set_url(rid, "file:///foo.wasm");
+                                  Deno.core.ops.op_wasm_streaming_feed(rid, source);
+                                  Deno.core.close(rid);
+                                });
+                               "#).unwrap();
+
+  let promise = runtime.execute_script_static("main.js",
+                            r#"
+                             // (module (import "env" "data" (global i64)))
+                             const bytes = new Uint8Array([0,97,115,109,1,0,0,0,2,13,1,3,101,110,118,4,100,97,116,97,3,126,0,0,8,4,110,97,109,101,2,1,0]);
+                             WebAssembly.instantiateStreaming(bytes, {
+                               env: {
+                                 get data() {
+                                   Deno.core.ops.op_resources();
+                                   return new WebAssembly.Global({ value: "i64", mutable: false }, 42n);
+                                 }
+                               }
+                             });
+                            "#).unwrap();
+  let value = runtime.resolve_value(promise).await.unwrap();
+  let val = value.open(&mut runtime.handle_scope());
+  assert!(val.is_object());
+}
+
 #[test]
 fn dangling_shared_isolate() {
   let v8_isolate_handle = {