Skip to content

Commit

Permalink
fixup! lib: allow CJS source map cache to be reclaimed
Browse files Browse the repository at this point in the history
  • Loading branch information
legendecas committed Mar 5, 2024
1 parent af592c5 commit 44c40aa
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 7 deletions.
15 changes: 9 additions & 6 deletions lib/internal/source_map/source_map_cache_map.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,22 @@ const {
ObjectFreeze,
SafeFinalizationRegistry,
SafeMap,
SafeWeakMap,
SafeWeakRef,
SymbolIterator,
} = primordials;
const {
privateSymbols: {
source_map_data_private_symbol,
},
} = internalBinding('util');

/**
* Specialized WeakMap that caches source map entries by `filename` and `sourceURL`.
* Cached entries can be iterated with `for..of`.
*
* The cache map maintains the cache entries by:
* - `weakTargetMap`(Map): a strong sourceURL -> WeakRef(Module),
* - `weakMap`(WeakMap): a Module instance object -> source map data.
* - WeakRef(Module[source_map_data_private_symbol]): source map data.
*
* Obsolete `weakTargetMap` entries are removed by the `finalizationRegistry` callback.
* This pattern decouples the strong url reference to the source map data and allow the
Expand All @@ -33,7 +37,6 @@ class SourceMapCacheMap {
* the cache by the `finalizationRegistry`.
*/
#weakTargetMap = new SafeMap();
#weakMap = new SafeWeakMap();

#cleanup = ({ keys }) => {
// Delete the entry if the weak target has been reclaimed.
Expand All @@ -58,7 +61,7 @@ class SourceMapCacheMap {
set(keys, value, weakTarget) {
const weakRef = new SafeWeakRef(weakTarget);
ArrayPrototypeForEach(keys, (key) => this.#weakTargetMap.set(key, weakRef));
this.#weakMap.set(weakTarget, value);
weakTarget[source_map_data_private_symbol] = value;
this.#finalizationRegistry.register(weakTarget, { keys });
}

Expand All @@ -72,7 +75,7 @@ class SourceMapCacheMap {
if (target === undefined) {
return;
}
return this.#weakMap.get(target);
return target[source_map_data_private_symbol];
}

/**
Expand All @@ -92,7 +95,7 @@ class SourceMapCacheMap {
const { 0: key, 1: weakRef } = result.value;
const target = weakRef.deref();
if (target == null) return next();
const value = this.#weakMap.get(target);
const value = target[source_map_data_private_symbol];
return { done: false, value: [key, value] };
};

Expand Down
3 changes: 2 additions & 1 deletion src/env_properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
V(untransferable_object_private_symbol, "node:untransferableObject") \
V(exit_info_private_symbol, "node:exit_info_private_symbol") \
V(promise_trace_id, "node:promise_trace_id") \
V(require_private_symbol, "node:require_private_symbol")
V(require_private_symbol, "node:require_private_symbol") \
V(source_map_data_private_symbol, "node:source_map_data_private_symbol")

// Symbols are per-isolate primitives but Environment proxies them
// for the sake of convenience.
Expand Down
8 changes: 8 additions & 0 deletions src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,14 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
return;
}

// Initialize an empty slot for source map cache before the object is frozen.
if (that->SetPrivate(context,
realm->isolate_data()->source_map_data_private_symbol(),
Undefined(isolate))
.IsNothing()) {
return;
}

// Use the extras object as an object whose GetCreationContext() will be the
// original `context`, since the `Context` itself strictly speaking cannot
// be stored in an internal field.
Expand Down

0 comments on commit 44c40aa

Please sign in to comment.