Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit bc2287d

Browse filesBrowse files
joyeecheungaduh95
authored andcommitted
src: add more checks and clarify docs for external references
To help catch unregistered bindings. PR-URL: #61719 Refs: #61718 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 4944358 commit bc2287d
Copy full SHA for bc2287d

2 files changed

+68-3Lines changed: 68 additions & 3 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎src/README.md‎

Copy file name to clipboardExpand all lines: src/README.md
+8-3Lines changed: 8 additions & 3 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -506,12 +506,17 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
506506
} // namespace util
507507
} // namespace node
508508
509-
// The first argument passed to `NODE_BINDING_EXTERNAL_REFERENCE`,
510-
// which is `util` here, needs to be added to the
511-
// `EXTERNAL_REFERENCE_BINDING_LIST_BASE` list in node_external_reference.h
512509
NODE_BINDING_EXTERNAL_REFERENCE(util, node::util::RegisterExternalReferences)
513510
```
514511

512+
And add the first argument passed to `NODE_BINDING_EXTERNAL_REFERENCE` to
513+
the list of external references in `src/node_external_reference.h`:
514+
515+
```cpp
516+
#define EXTERNAL_REFERENCE_LIST_BASE(V) \
517+
V(util) \
518+
```
519+
515520
Otherwise, you might see an error message like this when building the
516521
executables:
517522
Collapse file

‎src/node_snapshotable.cc‎

Copy file name to clipboardExpand all lines: src/node_snapshotable.cc
+60Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -968,6 +968,61 @@ std::optional<SnapshotConfig> ReadSnapshotConfig(const char* config_path) {
968968
return result;
969969
}
970970

971+
// Find bindings that have been loaded by internalBinding() but the external
972+
// reference method have not been called. This requires that the caller
973+
// match the id passed into their NODE_BINDING_CONTEXT_AWARE_INTERNAL() and
974+
// NODE_BINDING_EXTERNAL_REFERENCE() calls. Note that this only serves as a
975+
// preemptive check. Binding methods create the actual external references
976+
// (usually through function templates) and there's currently no easy way
977+
// to verify at that level of granularity. See "Registering binding functions
978+
// used in bootstrap" in src/README.md.
979+
bool ValidateBindings(Environment* env) {
980+
std::set<std::string> registered;
981+
#define V(modname) registered.insert(#modname);
982+
EXTERNAL_REFERENCE_BINDING_LIST(V)
983+
#undef V
984+
985+
std::set<std::string> bindings_without_external_references = {
986+
"async_context_frame",
987+
"constants",
988+
"symbols",
989+
};
990+
991+
std::set<std::string> unregistered;
992+
for (auto* mod : env->principal_realm()->internal_bindings) {
993+
if (registered.count(mod->nm_modname) == 0 &&
994+
bindings_without_external_references.count(mod->nm_modname) == 0) {
995+
unregistered.insert(mod->nm_modname);
996+
}
997+
}
998+
999+
if (unregistered.size() == 0) {
1000+
return true;
1001+
}
1002+
1003+
FPrintF(
1004+
stderr,
1005+
"\n---- snapshot building check failed ---\n\n"
1006+
"The following bindings are loaded during the snapshot building process,"
1007+
" but their external reference registration methods have not been "
1008+
"called:\n\n");
1009+
for (auto& binding : unregistered) {
1010+
FPrintF(stderr, " - %s\n", binding);
1011+
}
1012+
FPrintF(stderr,
1013+
"\nIf the binding does not have any external references, "
1014+
"add it to the list of bindings_without_external_references "
1015+
"in src/node_snapshotable.cc.\n"
1016+
"Otherwise, make sure to call NODE_BINDING_EXTERNAL_REFERENCE() "
1017+
"with an appropriate register method for the binding, "
1018+
"and add it to EXTERNAL_REFERENCE_BINDING_LIST in "
1019+
"src/node_external_reference.h"
1020+
"\n\nSee \"Registering binding functions used in bootstrap\" "
1021+
"in src/README.md for more details."
1022+
"\n----\n\n");
1023+
return false;
1024+
}
1025+
9711026
ExitCode BuildSnapshotWithoutCodeCache(
9721027
SnapshotData* out,
9731028
const std::vector<std::string>& args,
@@ -1033,6 +1088,11 @@ ExitCode BuildSnapshotWithoutCodeCache(
10331088
if (exit_code != ExitCode::kNoFailure) {
10341089
return exit_code;
10351090
}
1091+
1092+
if (snapshot_type == SnapshotMetadata::Type::kDefault &&
1093+
!ValidateBindings(env)) {
1094+
return ExitCode::kStartupSnapshotFailure;
1095+
}
10361096
}
10371097

10381098
return SnapshotBuilder::CreateSnapshot(out, setup.get());

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.