From d69aab62b0789dd54b8c09b54af022a38f060b5b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= <biwanczuk@gmail.com>
Date: Mon, 11 Mar 2024 22:18:03 +0000
Subject: fix(ext/node): make worker setup synchronous (#22815)

This commit fixes race condition in "node:worker_threads" module were
the first message did a setup of "threadId", "workerData" and
"environmentData".
Now this data is passed explicitly during workers creation and is set up
before any user code is executed.

Closes https://github.com/denoland/deno/issues/22783
Closes https://github.com/denoland/deno/issues/22672

---------

Co-authored-by: Satya Rohith <me@satyarohith.com>
---
 runtime/js/99_main.js      | 12 +++++++++++-
 runtime/ops/worker_host.rs |  3 +++
 runtime/web_worker.rs      | 15 ++++++++++++++-
 3 files changed, 28 insertions(+), 2 deletions(-)

(limited to 'runtime')

diff --git a/runtime/js/99_main.js b/runtime/js/99_main.js
index 82e444dfd..27ba488e7 100644
--- a/runtime/js/99_main.js
+++ b/runtime/js/99_main.js
@@ -786,6 +786,7 @@ function bootstrapWorkerRuntime(
   runtimeOptions,
   name,
   internalName,
+  maybeWorkerMetadata,
 ) {
   if (hasBootstrapped) {
     throw new Error("Worker runtime already bootstrapped");
@@ -908,8 +909,17 @@ function bootstrapWorkerRuntime(
   // existing global `Deno` with `Deno` namespace from "./deno.ts".
   ObjectDefineProperty(globalThis, "Deno", core.propReadOnly(finalDenoNs));
 
+  const workerMetadata = maybeWorkerMetadata
+    ? messagePort.deserializeJsMessageData(maybeWorkerMetadata)
+    : undefined;
+
   if (nodeBootstrap) {
-    nodeBootstrap(hasNodeModulesDir, argv0, /* runningOnMainThread */ false);
+    nodeBootstrap(
+      hasNodeModulesDir,
+      argv0,
+      /* runningOnMainThread */ false,
+      workerMetadata,
+    );
   }
 }
 
diff --git a/runtime/ops/worker_host.rs b/runtime/ops/worker_host.rs
index ee9f0dc5e..d1b318f0f 100644
--- a/runtime/ops/worker_host.rs
+++ b/runtime/ops/worker_host.rs
@@ -35,6 +35,7 @@ pub struct CreateWebWorkerArgs {
   pub permissions: PermissionsContainer,
   pub main_module: ModuleSpecifier,
   pub worker_type: WebWorkerType,
+  pub maybe_worker_metadata: Option<JsMessageData>,
 }
 
 pub type CreateWebWorkerCb = dyn Fn(CreateWebWorkerArgs) -> (WebWorker, SendableWebWorkerHandle)
@@ -121,6 +122,7 @@ pub struct CreateWorkerArgs {
 fn op_create_worker(
   state: &mut OpState,
   #[serde] args: CreateWorkerArgs,
+  #[serde] maybe_worker_metadata: Option<JsMessageData>,
 ) -> Result<WorkerId, AnyError> {
   let specifier = args.specifier.clone();
   let maybe_source_code = if args.has_source_code {
@@ -189,6 +191,7 @@ fn op_create_worker(
         permissions: worker_permissions,
         main_module: module_specifier.clone(),
         worker_type,
+        maybe_worker_metadata,
       });
 
     // Send thread safe handle from newly created worker to host thread
diff --git a/runtime/web_worker.rs b/runtime/web_worker.rs
index 6571da6c2..f35d38921 100644
--- a/runtime/web_worker.rs
+++ b/runtime/web_worker.rs
@@ -48,6 +48,7 @@ use deno_terminal::colors;
 use deno_tls::RootCertStoreProvider;
 use deno_web::create_entangled_message_port;
 use deno_web::BlobStore;
+use deno_web::JsMessageData;
 use deno_web::MessagePort;
 use log::debug;
 use std::cell::RefCell;
@@ -331,6 +332,8 @@ pub struct WebWorker {
   pub main_module: ModuleSpecifier,
   poll_for_messages_fn: Option<v8::Global<v8::Value>>,
   bootstrap_fn_global: Option<v8::Global<v8::Function>>,
+  // Consumed when `bootstrap_fn` is called
+  maybe_worker_metadata: Option<JsMessageData>,
 }
 
 pub struct WebWorkerOptions {
@@ -356,6 +359,7 @@ pub struct WebWorkerOptions {
   pub cache_storage_dir: Option<std::path::PathBuf>,
   pub stdio: Stdio,
   pub feature_checker: Arc<FeatureChecker>,
+  pub maybe_worker_metadata: Option<JsMessageData>,
 }
 
 impl WebWorker {
@@ -601,6 +605,7 @@ impl WebWorker {
         main_module,
         poll_for_messages_fn: None,
         bootstrap_fn_global: Some(bootstrap_fn_global),
+        maybe_worker_metadata: options.maybe_worker_metadata,
       },
       external_handle,
     )
@@ -616,6 +621,10 @@ impl WebWorker {
       let bootstrap_fn = self.bootstrap_fn_global.take().unwrap();
       let bootstrap_fn = v8::Local::new(scope, bootstrap_fn);
       let undefined = v8::undefined(scope);
+      let mut worker_data: v8::Local<v8::Value> = v8::undefined(scope).into();
+      if let Some(data) = self.maybe_worker_metadata.take() {
+        worker_data = deno_core::serde_v8::to_v8(scope, data).unwrap();
+      }
       let name_str: v8::Local<v8::Value> =
         v8::String::new(scope, &self.name).unwrap().into();
       let id_str: v8::Local<v8::Value> =
@@ -623,7 +632,11 @@ impl WebWorker {
           .unwrap()
           .into();
       bootstrap_fn
-        .call(scope, undefined.into(), &[args, name_str, id_str])
+        .call(
+          scope,
+          undefined.into(),
+          &[args, name_str, id_str, worker_data],
+        )
         .unwrap();
     }
     // TODO(bartlomieju): this could be done using V8 API, without calling `execute_script`.
-- 
cgit v1.2.3