Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support child_process in snapshot #50924

Closed
H4ad opened this issue Nov 26, 2023 · 5 comments
Closed

Support child_process in snapshot #50924

H4ad opened this issue Nov 26, 2023 · 5 comments
Labels
child_process Issues and PRs related to the child_process subsystem. feature request Issues that request new features to be added to Node.js. never-stale Mark issue so that it is never considered stale snapshot Issues and PRs related to the startup snapshot

Comments

@H4ad
Copy link
Member

H4ad commented Nov 26, 2023

Version

21.1.0

Platform

Linux h4ad 5.15.0-89-generic #99~20.04.1-Ubuntu SMP Thu Nov 2 15:16:47 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

child_process

What steps will reproduce the bug?

The following code:

// save as file.js
const child_process = require("node:child_process");
child_process.execSync("echo 1", { stdio: "inherit" });

Run with: node --snapshot-blob snapshot.blob --build-snapshot file.js

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior? Why is that the expected behavior?

The snapshot being created without core dumped.is

What do you see instead?

1
(node:136443) Warning: built-in module node:child_process is not yet supported in user snapshots
(Use `node --trace-warnings ...` to show where the warning was created)
Unknown external reference 0xf0e510.
<unresolved>
[1]    136443 trace trap (core dumped)  node --snapshot-blob snapshot.blob --build-snapshot file.js

Additional information

Found at https://github.com/mehulkar/bench-childprocess

@joyeecheung
Copy link
Member

I think at the very least we can register some of the child process methods. Some additional work may be done to ensure that users don't leave any ProcessWrap lingering when the snapshot is taken too.

@H4ad H4ad added confirmed-bug Issues with confirmed bugs. child_process Issues and PRs related to the child_process subsystem. snapshot Issues and PRs related to the startup snapshot labels Nov 27, 2023
@H4ad
Copy link
Member Author

H4ad commented Nov 28, 2023

@joyeecheung Is supposed for all unsupported modules to crash? I tried just require('cluster'), require('http') and I got some error/strange message.

(node:143236) Warning: built-in module cluster is not yet supported in user snapshots
(Use `node --trace-warnings ...` to show where the warning was created)
Unknown external reference 0xf0e510.
<unresolved>
(node:143589) Warning: built-in module http is not yet supported in user snapshots
(Use `node --trace-warnings ...` to show where the warning was created)
global handle not serialized: 0x43dc40a99f1: [[api object] 0] in OldSpace
 - map: 0x2a08a1ccfce9 <Map[40](HOLEY_ELEMENTS)> [FastProperties]
 - prototype: 0x25c423a8b9c1 <Object map = 0x3aa3b8ae20b9>
 - elements: 0x1c4597f00379 <FixedArray[0]> [HOLEY_ELEMENTS]
 - embedder fields: 2
 - properties: 0x043dc40ae701 <PropertyArray[3]>
 - All own properties (excluding elements): {
    0x25a483a72469: [String] in OldSpace: #methods: 0x25c423ab69e9 <JSArray[34]> (const data field 0), location: properties[0]
    0x2a08a1cc9519: [String] in OldSpace: #HTTPParser: 0x2a08a1cced31 <JSFunction HTTPParser (sfi = 0x2a08a1ccecf1)> (const data field 1), location: properties[1]
    0x2a08a1ccfcc9: [String] in OldSpace: #ConnectionsList: 0x2a08a1ccfc41 <JSFunction ConnectionsList (sfi = 0x2a08a1ccfc01)> (const data field 2), location: properties[2]
 }
 - embedder fields = {
    0, aligned pointer: 0x6c9b322
    0, aligned pointer: 0x6cea4b0
 }



#
# Fatal error in , line 0
# CheckGlobalAndEternalHandles failed
#
#
#
#FailureMessage Object: 0x7ffcb8661ae0
 1: 0xe5a541  [/home/h4ad/.asdf/installs/nodejs/21.1.0/bin/node]
 2: 0x22c7291 V8_Fatal(char const*, ...) [/home/h4ad/.asdf/installs/nodejs/21.1.0/bin/node]
 3: 0x169b14d v8::internal::SnapshotCreatorImpl::CreateBlob(v8::SnapshotCreator::FunctionCodeHandling, v8::base::Flags<v8::internal::Snapshot::SerializerFlag, int, int>) [/home/h4ad/.asdf/installs/nodejs/21.1.0/bin/node]
 4: 0xe99fd5 node::SnapshotBuilder::CreateSnapshot(node::SnapshotData*, node::CommonEnvironmentSetup*, unsigned char) [/home/h4ad/.asdf/installs/nodejs/21.1.0/bin/node]
 5: 0xe9a3c1 node::BuildSnapshotWithoutCodeCache(node::SnapshotData*, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&, std::optional<std::basic_string_view<char, std::char_traits<char> > >) [/home/h4ad/.asdf/installs/nodejs/21.1.0/bin/node]
 6: 0xe9bbd5 node::SnapshotBuilder::Generate(node::SnapshotData*, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&, std::optional<std::basic_string_view<char, std::char_traits<char> > >) [/home/h4ad/.asdf/installs/nodejs/21.1.0/bin/node]
 7: 0xd9a410 node::GenerateAndWriteSnapshotData(node::SnapshotData const**, node::InitializationResultImpl const*) [/home/h4ad/.asdf/installs/nodejs/21.1.0/bin/node]
 8: 0xd9a948 node::Start(int, char**) [/home/h4ad/.asdf/installs/nodejs/21.1.0/bin/node]
 9: 0x7f963d606083 __libc_start_main [/lib/x86_64-linux-gnu/libc.so.6]
10: 0xcf2c6e _start [/home/h4ad/.asdf/installs/nodejs/21.1.0/bin/node]
[1]    143589 trace trap (core dumped)  node --snapshot-blob snapshot.blob --build-snapshot empty-file.js

@joyeecheung
Copy link
Member

joyeecheung commented Nov 28, 2023

Is supposed for all unsupported modules to crash?

Not necessarily, they are crashing because they either have leftover wrapper objects that are not properly handled for the snapshot (could be a leak, even), or unregistered external references. Being unsupported means we haven't checked that they work in snapshot or make them to work in the snapshot, so they can fail in different ways, or work in some cases.

@joyeecheung joyeecheung changed the title core dumped on --build-snapshot with child_process Support child_process in snapshot Nov 30, 2023
@joyeecheung joyeecheung added feature request Issues that request new features to be added to Node.js. and removed confirmed-bug Issues with confirmed bugs. labels Nov 30, 2023
joyeecheung added a commit that referenced this issue Dec 14, 2023
PR-URL: #50943
Refs: #50924
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
joyeecheung added a commit that referenced this issue Dec 14, 2023
PR-URL: #50943
Refs: #50924
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
joyeecheung added a commit that referenced this issue Dec 14, 2023
PR-URL: #50943
Refs: #50924
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
joyeecheung added a commit that referenced this issue Dec 14, 2023
These currently work in snapshot builder scripts. Asynchronous
methods are not supported yet.

PR-URL: #50943
Refs: #50924
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
RafaelGSS pushed a commit that referenced this issue Dec 15, 2023
PR-URL: #50943
Refs: #50924
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
RafaelGSS pushed a commit that referenced this issue Dec 15, 2023
PR-URL: #50943
Refs: #50924
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
RafaelGSS pushed a commit that referenced this issue Dec 15, 2023
PR-URL: #50943
Refs: #50924
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
RafaelGSS pushed a commit that referenced this issue Dec 15, 2023
These currently work in snapshot builder scripts. Asynchronous
methods are not supported yet.

PR-URL: #50943
Refs: #50924
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
richardlau pushed a commit that referenced this issue Mar 25, 2024
PR-URL: #50943
Refs: #50924
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
richardlau pushed a commit that referenced this issue Mar 25, 2024
PR-URL: #50943
Refs: #50924
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
richardlau pushed a commit that referenced this issue Mar 25, 2024
PR-URL: #50943
Refs: #50924
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
richardlau pushed a commit that referenced this issue Mar 25, 2024
These currently work in snapshot builder scripts. Asynchronous
methods are not supported yet.

PR-URL: #50943
Refs: #50924
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Copy link
Contributor

There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the never-stale Mark issue so that it is never considered stale label or close this issue if it should be closed. If not, the issue will be automatically closed 6 months after the last non-automated comment.
For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label May 29, 2024
@H4ad H4ad added never-stale Mark issue so that it is never considered stale and removed stale labels May 29, 2024
@joyeecheung
Copy link
Member

The synchronous methods are supported since #50943 so the snippet in the OP works now and I am closing this. Asynchronous method support would be a different beast (I think as long as the method finishes and resources are cleaned up before snapshot is taken it should work, haven't verified it yet though).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
child_process Issues and PRs related to the child_process subsystem. feature request Issues that request new features to be added to Node.js. never-stale Mark issue so that it is never considered stale snapshot Issues and PRs related to the startup snapshot
Projects
Status: Awaiting Triage
Development

No branches or pull requests

2 participants