fix: initialize asar support in worker threads #33216

indutny · 2022-03-10T02:03:37Z

Description of Change

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.

Test case: github.com/indutny-signal/asar-worker

cc @codebytere

Checklist

PR description included and stakeholders cc'd
npm test passes
tests are changed or added
relevant documentation is changed or added
PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes: 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.

RaisinTen

Would it make sense to add the test to spec/asar-spec.js and include the asar in spec/fixtures?

lib/asar/fs-wrapper.ts

patches/node/worker_thread_add_asar_support.patch

Co-authored-by: Darshan Sen <raisinten@gmail.com>

indutny-signal · 2022-03-10T07:30:53Z

@RaisinTen good point. Added a test into spec-main's asar test suite. Thank you!

indutny-signal · 2022-03-10T20:03:26Z

I suppose I can also cc @zcbenz , not sure who the code owner is for this part of the code.

indutny-signal · 2022-03-14T20:50:21Z

(Friendly ping to keep the notifications up in inbox)

indutny-signal · 2022-03-21T17:05:28Z

Awesome. Thanks for approving! Do we need one more approval to merge it @zcbenz ?

zcbenz · 2022-03-21T23:47:06Z

I prefer waiting for one more approval, but if there is none I think it is ok to just merge this.

indutny · 2022-03-22T00:32:20Z

cc @MarshallOfSound @nornagon and just in case @codebytere

release-clerk · 2022-03-23T00:37:58Z

Release Notes Persisted

fix: initialize asar support in worker threads

* 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 <raisinten@gmail.com> * Update patches/node/worker_thread_add_asar_support.patch Co-authored-by: Darshan Sen <raisinten@gmail.com> * Add a test Co-authored-by: Darshan Sen <raisinten@gmail.com> Co-authored-by: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>

trop · 2022-03-23T00:38:25Z

I have automatically backported this PR to "17-x-y", please check out #33395

trop · 2022-03-23T00:38:28Z

I have automatically backported this PR to "18-x-y", please check out #33396

indutny-signal · 2022-03-23T00:43:14Z

Thank you!

* 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 <raisinten@gmail.com> * Update patches/node/worker_thread_add_asar_support.patch Co-authored-by: Darshan Sen <raisinten@gmail.com> * Add a test Co-authored-by: Darshan Sen <raisinten@gmail.com> Co-authored-by: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org> * chore: update .patches after merge Co-authored-by: Fedor Indutny <fedor@indutny.com> Co-authored-by: Darshan Sen <raisinten@gmail.com> Co-authored-by: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>

* 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 <raisinten@gmail.com> * Update patches/node/worker_thread_add_asar_support.patch Co-authored-by: Darshan Sen <raisinten@gmail.com> * Add a test Co-authored-by: Darshan Sen <raisinten@gmail.com> Co-authored-by: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org> * chore: update patches Co-authored-by: Fedor Indutny <fedor@indutny.com> Co-authored-by: Darshan Sen <raisinten@gmail.com> Co-authored-by: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org> Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>

* 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 <raisinten@gmail.com> * Update patches/node/worker_thread_add_asar_support.patch Co-authored-by: Darshan Sen <raisinten@gmail.com> * Add a test Co-authored-by: Darshan Sen <raisinten@gmail.com> Co-authored-by: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>

参见electron/electron#33216 electron已经支持从asar中启动worker threads

indutny requested review from a team as code owners March 10, 2022 02:03

electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Mar 10, 2022

indutny-signal force-pushed the feature/asar-support-in-worker-threads branch from ccb5b37 to 98bc950 Compare March 10, 2022 02:15

indutny force-pushed the feature/asar-support-in-worker-threads branch from 98bc950 to 3a05f9c Compare March 10, 2022 02:51

RaisinTen reviewed Mar 10, 2022

View reviewed changes

lib/asar/fs-wrapper.ts Outdated Show resolved Hide resolved

patches/node/worker_thread_add_asar_support.patch Outdated Show resolved Hide resolved

indutny and others added 3 commits March 9, 2022 22:26

Update lib/asar/fs-wrapper.ts

51aaabc

Co-authored-by: Darshan Sen <raisinten@gmail.com>

Update patches/node/worker_thread_add_asar_support.patch

897e082

Co-authored-by: Darshan Sen <raisinten@gmail.com>

Add a test

7ce6f72

Merge branch 'main' into feature/asar-support-in-worker-threads

49203e4

electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Mar 17, 2022

zcbenz approved these changes Mar 21, 2022

View reviewed changes

zcbenz added semver/patch backwards-compatible bug fixes target/17-x-y labels Mar 21, 2022

Merge branch 'main' into feature/asar-support-in-worker-threads

b631fcd

jkleinsc approved these changes Mar 23, 2022

View reviewed changes

jkleinsc merged commit 06a00b7 into electron:main Mar 23, 2022

trop bot mentioned this pull request Mar 23, 2022

fix: initialize asar support in worker threads #33395

Merged

trop bot mentioned this pull request Mar 23, 2022

fix: initialize asar support in worker threads #33396

Merged

trop bot added in-flight/17-x-y and removed target/17-x-y labels Mar 23, 2022

trop bot added in-flight/18-x-y and removed target/18-x-y labels Mar 23, 2022

indutny-signal deleted the feature/asar-support-in-worker-threads branch March 23, 2022 00:43

trop bot added merged/18-x-y and removed in-flight/18-x-y labels Mar 24, 2022

trop bot removed the in-flight/17-x-y label Mar 24, 2022

hgossler mentioned this pull request Apr 5, 2022

Support spawning threads from electron ASAR file andywer/threads.js#425

Open

commit-lint bot mentioned this pull request May 11, 2022

[pull] main from electron:main Blaquez-home/electron#3

Open

toyobayashi mentioned this pull request Sep 25, 2022

fix: recursion error (max call stack size exceeded) toyobayashi/asar-node#6

Merged

hrueger mentioned this pull request Dec 2, 2022

Not working in packed electron >=17.3 nytamin/threadedClass#74

Closed

This was referenced Dec 8, 2022

[Bug]: memory leak when a worker accesses a file in the asar archive package for the first time #36597

Closed

fix: use the process cache to reduce the memory for asar file #36600

Merged

nzh63 added a commit to nzh63/Ame that referenced this pull request Oct 22, 2023

refactor: 优化打包，尽可能将数据放入asar

4c56053

参见electron/electron#33216 electron已经支持从asar中启动worker threads

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: initialize asar support in worker threads #33216

fix: initialize asar support in worker threads #33216

indutny commented Mar 10, 2022 •

edited

RaisinTen left a comment

indutny-signal commented Mar 10, 2022 •

edited

indutny-signal commented Mar 10, 2022

indutny-signal commented Mar 14, 2022

indutny-signal commented Mar 21, 2022

zcbenz commented Mar 21, 2022

indutny commented Mar 22, 2022

release-clerk bot commented Mar 23, 2022

trop bot commented Mar 23, 2022

trop bot commented Mar 23, 2022

indutny-signal commented Mar 23, 2022

fix: initialize asar support in worker threads #33216

fix: initialize asar support in worker threads #33216

Conversation

indutny commented Mar 10, 2022 • edited

Description of Change

Checklist

Release Notes

RaisinTen left a comment

Choose a reason for hiding this comment

indutny-signal commented Mar 10, 2022 • edited

indutny-signal commented Mar 10, 2022

indutny-signal commented Mar 14, 2022

indutny-signal commented Mar 21, 2022

zcbenz commented Mar 21, 2022

indutny commented Mar 22, 2022

release-clerk bot commented Mar 23, 2022

trop bot commented Mar 23, 2022

trop bot commented Mar 23, 2022

indutny-signal commented Mar 23, 2022

indutny commented Mar 10, 2022 •

edited

indutny-signal commented Mar 10, 2022 •

edited