Skip to content

Commit 2e86729

Browse files
authored
fix(shell-evaluator): do not let eval() hold extra context MONGOSH-3223 (#2677)
Calling `eval()` makes the entire outside scope available to the evaluated code, and consequentially keeps it alive for garbage collection purposes longer than it should or than it needs to. Since we only use `eval()` to modify global state, putting the call into a file where no other extra context exists is a decent solution to this issue.
1 parent 3b86998 commit 2e86729

File tree

2 files changed

+17
-3
lines changed

2 files changed

+17
-3
lines changed
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// This may seem like a pointless function, but eval() in JS
2+
// is tricky. If we were to call eval() from inside a function
3+
// in another file directly, the variables in that function would
4+
// affect and be available to the eval'ed source, which can
5+
// generally cause some weirdness in evaluating the inner code,
6+
// but also in particular establishes a garbage collection link
7+
// between the eval'ed source and those variables.
8+
// Putting this function in a separate file ensures that no
9+
// unintentional references to other variables can be captured.
10+
export function contextlessEval(code: string): void {
11+
eval(code);
12+
}

packages/shell-evaluator/src/shell-evaluator.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { contextlessEval } from './contextless-eval';
12
import type { ShellInstanceState } from '@mongosh/shell-api';
23
import {
34
toShellResult,
@@ -26,8 +27,8 @@ try {
2627
if (v8?.startupSnapshot?.isBuildingSnapshot?.()) {
2728
v8.startupSnapshot.addSerializeCallback(() => {
2829
// Ensure that any lazy loading performed by Babel is part of the snapshot
29-
eval(new AsyncWriter().runtimeSupportCode());
30-
eval(new AsyncWriter().process('1+1'));
30+
contextlessEval(new AsyncWriter().runtimeSupportCode());
31+
contextlessEval(new AsyncWriter().process('1+1'));
3132
hasAlreadyRunGlobalRuntimeSupportEval = true;
3233
});
3334
}
@@ -120,7 +121,8 @@ class ShellEvaluator<EvaluationResultType = ShellResult> {
120121
// in which the shell-api package lives and not from the context inside
121122
// the REPL (i.e. `db.test.find().toArray() instanceof Array` is `false`).
122123
if (!hasAlreadyRunGlobalRuntimeSupportEval) {
123-
eval(supportCode);
124+
contextlessEval(supportCode);
125+
hasAlreadyRunGlobalRuntimeSupportEval = true;
124126
}
125127
this.markTime?.(
126128
TimingCategories.AsyncRewrite,

0 commit comments

Comments
 (0)