Browse Source

fix: initialize asar support in worker threads (#33216)

* fix: initialize asar support in worker threads

Use `ObjectWrap` instead of gin's Wrap in `electron_api_asar.cc` because
gin isn't fully initialized (and apparently not possible to initialize
without ruining the isolate configuration and array buffer allocator) in
worker threads. In the worker thread call `setupAsarSupport` just as we
do for the main process.

* Update lib/asar/fs-wrapper.ts

Co-authored-by: Darshan Sen <[email protected]>

* Update patches/node/worker_thread_add_asar_support.patch

Co-authored-by: Darshan Sen <[email protected]>

* Add a test

Co-authored-by: Darshan Sen <[email protected]>
Co-authored-by: Fedor Indutny <[email protected]>
Co-authored-by: John Kleinschmidt <[email protected]>
Fedor Indutny 3 years ago
parent
commit
06a00b74e8

+ 7 - 5
lib/asar/fs-wrapper.ts

@@ -30,11 +30,13 @@ const getOrCreateArchive = (archivePath: string) => {
     return cachedArchives.get(archivePath);
   }
 
-  const newArchive = asar.createArchive(archivePath);
-  if (!newArchive) return null;
-
-  cachedArchives.set(archivePath, newArchive);
-  return newArchive;
+  try {
+    const newArchive = new asar.Archive(archivePath);
+    cachedArchives.set(archivePath, newArchive);
+    return newArchive;
+  } catch {
+    return null;
+  }
 };
 
 const asarRe = /\.asar/i;

+ 1 - 0
patches/node/.patches

@@ -34,3 +34,4 @@ darwin_bump_minimum_supported_version_to_10_15_3406.patch
 fix_failing_node_js_test_on_outdated.patch
 be_compatible_with_cppgc.patch
 feat_add_knostartdebugsignalhandler_to_environment_to_prevent.patch
+worker_thread_add_asar_support.patch

+ 41 - 0
patches/node/worker_thread_add_asar_support.patch

@@ -0,0 +1,41 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Fedor Indutny <[email protected]>
+Date: Wed, 9 Mar 2022 17:52:32 -0800
+Subject: worker_thread: add asar support
+
+This patch initializes asar support in workers threads in
+Node.js.
+
+diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js
+index 419ffd9d5deb84eb94381259d3084411f6c3341b..17a1860d158976f11035553601560d171c7fc25a 100644
+--- a/lib/internal/bootstrap/pre_execution.js
++++ b/lib/internal/bootstrap/pre_execution.js
+@@ -505,6 +505,7 @@ module.exports = {
+   loadPreloadModules,
+   setupTraceCategoryState,
+   setupInspectorHooks,
++  setupAsarSupport,
+   initializeReport,
+   initializeCJSLoader,
+   initializeWASI
+diff --git a/lib/internal/main/worker_thread.js b/lib/internal/main/worker_thread.js
+index e3ce67987ee3185a93750ebad72beab304c71e3a..ef5082d73b6153b49875c61d9b365b873b16145d 100644
+--- a/lib/internal/main/worker_thread.js
++++ b/lib/internal/main/worker_thread.js
+@@ -27,6 +27,7 @@ const {
+   initializeReport,
+   initializeSourceMapsHandlers,
+   loadPreloadModules,
++  setupAsarSupport,
+   setupTraceCategoryState
+ } = require('internal/bootstrap/pre_execution');
+ 
+@@ -154,6 +155,8 @@ port.on('message', (message) => {
+     };
+     workerIo.sharedCwdCounter = cwdCounter;
+ 
++    setupAsarSupport();
++
+     const CJSLoader = require('internal/modules/cjs/loader');
+     assert(!CJSLoader.hasLoadedAnyUserCJSModule);
+     loadPreloadModules();

+ 143 - 65
shell/common/api/electron_api_asar.cc

@@ -5,11 +5,8 @@
 #include <vector>
 
 #include "gin/handle.h"
-#include "gin/object_template_builder.h"
-#include "gin/wrappable.h"
 #include "shell/common/asar/archive.h"
 #include "shell/common/asar/asar_util.h"
-#include "shell/common/gin_converters/callback_converter.h"
 #include "shell/common/gin_converters/file_path_converter.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/node_includes.h"
@@ -17,45 +14,73 @@
 
 namespace {
 
-class Archive : public gin::Wrappable<Archive> {
+class Archive : public node::ObjectWrap {
  public:
-  static gin::Handle<Archive> Create(v8::Isolate* isolate,
-                                     const base::FilePath& path) {
-    auto archive = std::make_unique<asar::Archive>(path);
-    if (!archive->Init())
-      return gin::Handle<Archive>();
-    return gin::CreateHandle(isolate, new Archive(isolate, std::move(archive)));
-  }
-
-  // gin::Wrappable
-  static gin::WrapperInfo kWrapperInfo;
-  gin::ObjectTemplateBuilder GetObjectTemplateBuilder(
-      v8::Isolate* isolate) override {
-    return gin::ObjectTemplateBuilder(isolate)
-        .SetMethod("getFileInfo", &Archive::GetFileInfo)
-        .SetMethod("stat", &Archive::Stat)
-        .SetMethod("readdir", &Archive::Readdir)
-        .SetMethod("realpath", &Archive::Realpath)
-        .SetMethod("copyFileOut", &Archive::CopyFileOut)
-        .SetMethod("getFdAndValidateIntegrityLater", &Archive::GetFD);
+  static v8::Local<v8::FunctionTemplate> CreateFunctionTemplate(
+      v8::Isolate* isolate) {
+    auto tpl = v8::FunctionTemplate::New(isolate, Archive::New);
+    tpl->SetClassName(
+        v8::String::NewFromUtf8(isolate, "Archive").ToLocalChecked());
+    tpl->InstanceTemplate()->SetInternalFieldCount(1);
+
+    NODE_SET_PROTOTYPE_METHOD(tpl, "getFileInfo", &Archive::GetFileInfo);
+    NODE_SET_PROTOTYPE_METHOD(tpl, "stat", &Archive::Stat);
+    NODE_SET_PROTOTYPE_METHOD(tpl, "readdir", &Archive::Readdir);
+    NODE_SET_PROTOTYPE_METHOD(tpl, "realpath", &Archive::Realpath);
+    NODE_SET_PROTOTYPE_METHOD(tpl, "copyFileOut", &Archive::CopyFileOut);
+    NODE_SET_PROTOTYPE_METHOD(tpl, "getFdAndValidateIntegrityLater",
+                              &Archive::GetFD);
+
+    return tpl;
   }
 
-  const char* GetTypeName() override { return "Archive"; }
-
   // disable copy
   Archive(const Archive&) = delete;
   Archive& operator=(const Archive&) = delete;
 
  protected:
-  Archive(v8::Isolate* isolate, std::unique_ptr<asar::Archive> archive)
+  explicit Archive(std::unique_ptr<asar::Archive> archive)
       : archive_(std::move(archive)) {}
 
+  static void New(const v8::FunctionCallbackInfo<v8::Value>& args) {
+    auto* isolate = args.GetIsolate();
+
+    base::FilePath path;
+    if (!gin::ConvertFromV8(isolate, args[0], &path)) {
+      isolate->ThrowException(v8::Exception::Error(node::FIXED_ONE_BYTE_STRING(
+          isolate, "failed to convert path to V8")));
+      return;
+    }
+
+    auto archive = std::make_unique<asar::Archive>(path);
+    if (!archive->Init()) {
+      isolate->ThrowException(v8::Exception::Error(node::FIXED_ONE_BYTE_STRING(
+          isolate, "failed to initialize archive")));
+      return;
+    }
+
+    auto* archive_wrap = new Archive(std::move(archive));
+    archive_wrap->Wrap(args.This());
+    args.GetReturnValue().Set(args.This());
+  }
+
   // Reads the offset and size of file.
-  v8::Local<v8::Value> GetFileInfo(v8::Isolate* isolate,
-                                   const base::FilePath& path) {
+  static void GetFileInfo(const v8::FunctionCallbackInfo<v8::Value>& args) {
+    auto* isolate = args.GetIsolate();
+    auto* wrap = node::ObjectWrap::Unwrap<Archive>(args.Holder());
+
+    base::FilePath path;
+    if (!gin::ConvertFromV8(isolate, args[0], &path)) {
+      args.GetReturnValue().Set(v8::False(isolate));
+      return;
+    }
+
     asar::Archive::FileInfo info;
-    if (!archive_ || !archive_->GetFileInfo(path, &info))
-      return v8::False(isolate);
+    if (!wrap->archive_ || !wrap->archive_->GetFileInfo(path, &info)) {
+      args.GetReturnValue().Set(v8::False(isolate));
+      return;
+    }
+
     gin_helper::Dictionary dict(isolate, v8::Object::New(isolate));
     dict.Set("size", info.size);
     dict.Set("unpacked", info.unpacked);
@@ -74,65 +99,104 @@ class Archive : public gin::Wrappable<Archive> {
       integrity.Set("hash", info.integrity.value().hash);
       dict.Set("integrity", integrity);
     }
-    return dict.GetHandle();
+    args.GetReturnValue().Set(dict.GetHandle());
   }
 
   // Returns a fake result of fs.stat(path).
-  v8::Local<v8::Value> Stat(v8::Isolate* isolate, const base::FilePath& path) {
+  static void Stat(const v8::FunctionCallbackInfo<v8::Value>& args) {
+    auto* isolate = args.GetIsolate();
+    auto* wrap = node::ObjectWrap::Unwrap<Archive>(args.Holder());
+    base::FilePath path;
+    if (!gin::ConvertFromV8(isolate, args[0], &path)) {
+      args.GetReturnValue().Set(v8::False(isolate));
+      return;
+    }
+
     asar::Archive::Stats stats;
-    if (!archive_ || !archive_->Stat(path, &stats))
-      return v8::False(isolate);
+    if (!wrap->archive_ || !wrap->archive_->Stat(path, &stats)) {
+      args.GetReturnValue().Set(v8::False(isolate));
+      return;
+    }
+
     gin_helper::Dictionary dict(isolate, v8::Object::New(isolate));
     dict.Set("size", stats.size);
     dict.Set("offset", stats.offset);
     dict.Set("isFile", stats.is_file);
     dict.Set("isDirectory", stats.is_directory);
     dict.Set("isLink", stats.is_link);
-    return dict.GetHandle();
+    args.GetReturnValue().Set(dict.GetHandle());
   }
 
   // Returns all files under a directory.
-  v8::Local<v8::Value> Readdir(v8::Isolate* isolate,
-                               const base::FilePath& path) {
+  static void Readdir(const v8::FunctionCallbackInfo<v8::Value>& args) {
+    auto* isolate = args.GetIsolate();
+    auto* wrap = node::ObjectWrap::Unwrap<Archive>(args.Holder());
+    base::FilePath path;
+    if (!gin::ConvertFromV8(isolate, args[0], &path)) {
+      args.GetReturnValue().Set(v8::False(isolate));
+      return;
+    }
+
     std::vector<base::FilePath> files;
-    if (!archive_ || !archive_->Readdir(path, &files))
-      return v8::False(isolate);
-    return gin::ConvertToV8(isolate, files);
+    if (!wrap->archive_ || !wrap->archive_->Readdir(path, &files)) {
+      args.GetReturnValue().Set(v8::False(isolate));
+      return;
+    }
+    args.GetReturnValue().Set(gin::ConvertToV8(isolate, files));
   }
 
   // Returns the path of file with symbol link resolved.
-  v8::Local<v8::Value> Realpath(v8::Isolate* isolate,
-                                const base::FilePath& path) {
+  static void Realpath(const v8::FunctionCallbackInfo<v8::Value>& args) {
+    auto* isolate = args.GetIsolate();
+    auto* wrap = node::ObjectWrap::Unwrap<Archive>(args.Holder());
+    base::FilePath path;
+    if (!gin::ConvertFromV8(isolate, args[0], &path)) {
+      args.GetReturnValue().Set(v8::False(isolate));
+      return;
+    }
+
     base::FilePath realpath;
-    if (!archive_ || !archive_->Realpath(path, &realpath))
-      return v8::False(isolate);
-    return gin::ConvertToV8(isolate, realpath);
+    if (!wrap->archive_ || !wrap->archive_->Realpath(path, &realpath)) {
+      args.GetReturnValue().Set(v8::False(isolate));
+      return;
+    }
+    args.GetReturnValue().Set(gin::ConvertToV8(isolate, realpath));
   }
 
   // Copy the file out into a temporary file and returns the new path.
-  v8::Local<v8::Value> CopyFileOut(v8::Isolate* isolate,
-                                   const base::FilePath& path) {
+  static void CopyFileOut(const v8::FunctionCallbackInfo<v8::Value>& args) {
+    auto* isolate = args.GetIsolate();
+    auto* wrap = node::ObjectWrap::Unwrap<Archive>(args.Holder());
+    base::FilePath path;
+    if (!gin::ConvertFromV8(isolate, args[0], &path)) {
+      args.GetReturnValue().Set(v8::False(isolate));
+      return;
+    }
+
     base::FilePath new_path;
-    if (!archive_ || !archive_->CopyFileOut(path, &new_path))
-      return v8::False(isolate);
-    return gin::ConvertToV8(isolate, new_path);
+    if (!wrap->archive_ || !wrap->archive_->CopyFileOut(path, &new_path)) {
+      args.GetReturnValue().Set(v8::False(isolate));
+      return;
+    }
+    args.GetReturnValue().Set(gin::ConvertToV8(isolate, new_path));
   }
 
   // Return the file descriptor.
-  int GetFD() const {
-    if (!archive_)
-      return -1;
-    return archive_->GetUnsafeFD();
+  static void GetFD(const v8::FunctionCallbackInfo<v8::Value>& args) {
+    auto* isolate = args.GetIsolate();
+    auto* wrap = node::ObjectWrap::Unwrap<Archive>(args.Holder());
+
+    args.GetReturnValue().Set(gin::ConvertToV8(
+        isolate, wrap->archive_ ? wrap->archive_->GetUnsafeFD() : -1));
   }
 
- private:
   std::unique_ptr<asar::Archive> archive_;
 };
 
-// static
-gin::WrapperInfo Archive::kWrapperInfo = {gin::kEmbedderNativeGin};
+static void InitAsarSupport(const v8::FunctionCallbackInfo<v8::Value>& args) {
+  auto* isolate = args.GetIsolate();
+  auto require = args[0];
 
-void InitAsarSupport(v8::Isolate* isolate, v8::Local<v8::Value> require) {
   // Evaluate asar_bundle.js.
   std::vector<v8::Local<v8::String>> asar_bundle_params = {
       node::FIXED_ONE_BYTE_STRING(isolate, "require")};
@@ -142,8 +206,15 @@ void InitAsarSupport(v8::Isolate* isolate, v8::Local<v8::Value> require) {
       &asar_bundle_params, &asar_bundle_args, nullptr);
 }
 
-v8::Local<v8::Value> SplitPath(v8::Isolate* isolate,
-                               const base::FilePath& path) {
+static void SplitPath(const v8::FunctionCallbackInfo<v8::Value>& args) {
+  auto* isolate = args.GetIsolate();
+
+  base::FilePath path;
+  if (!gin::ConvertFromV8(isolate, args[0], &path)) {
+    args.GetReturnValue().Set(v8::False(isolate));
+    return;
+  }
+
   gin_helper::Dictionary dict = gin::Dictionary::CreateEmpty(isolate);
   base::FilePath asar_path, file_path;
   if (asar::GetAsarArchivePath(path, &asar_path, &file_path, true)) {
@@ -153,17 +224,24 @@ v8::Local<v8::Value> SplitPath(v8::Isolate* isolate,
   } else {
     dict.Set("isAsar", false);
   }
-  return dict.GetHandle();
+  args.GetReturnValue().Set(dict.GetHandle());
 }
 
 void Initialize(v8::Local<v8::Object> exports,
                 v8::Local<v8::Value> unused,
                 v8::Local<v8::Context> context,
                 void* priv) {
-  gin_helper::Dictionary dict(context->GetIsolate(), exports);
-  dict.SetMethod("createArchive", &Archive::Create);
-  dict.SetMethod("splitPath", &SplitPath);
-  dict.SetMethod("initAsarSupport", &InitAsarSupport);
+  auto* isolate = exports->GetIsolate();
+
+  auto cons = Archive::CreateFunctionTemplate(isolate)
+                  ->GetFunction(context)
+                  .ToLocalChecked();
+  cons->SetName(node::FIXED_ONE_BYTE_STRING(isolate, "Archive"));
+
+  exports->Set(context, node::FIXED_ONE_BYTE_STRING(isolate, "Archive"), cons)
+      .Check();
+  NODE_SET_METHOD(exports, "splitPath", &SplitPath);
+  NODE_SET_METHOD(exports, "initAsarSupport", &InitAsarSupport);
 }
 
 }  // namespace

+ 1 - 0
shell/common/node_includes.h

@@ -23,6 +23,7 @@
 #include "node_errors.h"
 #include "node_internals.h"
 #include "node_native_module_env.h"
+#include "node_object_wrap.h"
 #include "node_options-inl.h"
 #include "node_options.h"
 #include "node_platform.h"

+ 16 - 0
spec-main/asar-spec.ts

@@ -1,6 +1,7 @@
 import { expect } from 'chai';
 import * as path from 'path';
 import * as url from 'url';
+import { Worker } from 'worker_threads';
 import { BrowserWindow, ipcMain } from 'electron/main';
 import { closeAllWindows } from './window-helpers';
 import { emittedOnce } from './events-helpers';
@@ -108,4 +109,19 @@ describe('asar package', () => {
       expect(result).to.equal('success');
     });
   });
+
+  describe('worker threads', function () {
+    it('should start worker thread from asar file', function (callback) {
+      const p = path.join(asarDir, 'worker_threads.asar', 'worker.js');
+      const w = new Worker(p);
+
+      w.on('error', (err) => callback(err));
+      w.on('message', (message) => {
+        expect(message).to.equal('ping');
+        w.terminate();
+
+        callback(null);
+      });
+    });
+  });
 });

BIN
spec/fixtures/test.asar/worker_threads.asar


+ 1 - 1
typings/internal-ambient.d.ts

@@ -83,7 +83,7 @@ declare namespace NodeJS {
   }
 
   interface AsarBinding {
-    createArchive(path: string): AsarArchive;
+    Archive: { new(path: string): AsarArchive };
     splitPath(path: string): {
       isAsar: false;
     } | {