sea: only assert snapshot main function for main threads

[joyeecheung]

Snapshot main functions are only loaded for main threads in single executable applications. Update the check to avoid asserting it in worker threads - this allows worker threads to be spawned in snapshot main functions bundled into a single executable application.

(Also noticed that worker_threads cannot be loaded in the snapshot builder script, which is to be expected currently since we haven't vetted the worker_threads module for snapshot building. I left a FIXME on how to vet the worker internals and make it loadable in the snapshot builder).

Fixes: #56077

[nodejs-github-bot]

Review requested:

• @nodejs/single-executable
• @nodejs/startup

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/63850/

[codecov]

Codecov Report

Attention: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 87.98%. Comparing base (61e4ad5) to head (630b6f8).
Report is 41 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node.cc	50.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #56120      +/-   ##
==========================================
- Coverage   88.01%   87.98%   -0.03%     
==========================================
  Files         656      656              
  Lines      188982   188960      -22     
  Branches    35997    35978      -19     
==========================================
- Hits       166330   166263      -67     
- Misses      15825    15861      +36     
- Partials     6827     6836       +9     

Files with missing lines	Coverage Δ
src/node.cc	73.50% <50.00%> (-0.11%)	⬇️

... and 50 files with indirect coverage changes

[legendecas]

Can we add a check that env->snapshot_deserialize_main().IsEmpty() when it is not the main thread?

IIUC this relies on the trick that context->GetDataFromSnapshotOnce will only return the data once and the second time it will return empty, as the main thread and worker threads are using the same snapshot data. Edit: the worker's main context uses SnapshotData::kNodeBaseContextIndex which has no snapshot_deserialize_main. But I think it is still worth adding the check to ensure snapshot_deserialize_main is not set on worker threads, for the lines below:

node/src/node.cc

Lines 344 to 346 in fe12b01

	if (!env->snapshot_deserialize_main().IsEmpty()) {
	return env->RunSnapshotDeserializeMain();
	}

[joyeecheung]

hmm yeah, we still haven't figured out how it should really work for workers - we'll likely need some restructuring of the snapshot blob/API. It currently goes like:

• built-in default context with only stuff from V8
• built-in vm context with the interceptor template and some fixups to the globals
• built-in worker context which currently only runs through NewContext()
• built-in/custom main context which runs through CreateEnvironment()

All these share one isolate context.

It's still possible to support custom worker context snapshot (in which case the main context is the "root" worker context) + populate more stuff in built-in worker context snapshot, but it will probably need some restructuring. For a fuller built-in worker context snapshot we'll need to run an additional CreateEnvironment() call on the same isolate but with different flags and do this

node/src/env.h

Line 568 in a243225

// TODO(joyeecheung): there should be a vector of env_info once we snapshot

, though I am not 100% sure if it works properly at the moment...can take a look as I look into the worker + snapshot combos a bit. For now a check to ensure snapshot_deserialize_main is not set on worker threads is fine.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/63873/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/63899/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/63914/

[nodejs-github-bot]

Landed in 7fdeeac