Skip to content

fix: loop-guard scope leak #4062

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 29 additions & 35 deletions src/compiler/compile/Component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -344,15 +344,16 @@ export default class Component {
};
}

get_unique_name(name: string): Identifier {
get_unique_name(name: string, scope?: Scope): Identifier {
if (test) name = `${name}$`;
let alias = name;
for (
let i = 1;
reserved.has(alias) ||
this.var_lookup.has(alias) ||
this.used_names.has(alias) ||
this.globally_used_names.has(alias);
this.globally_used_names.has(alias) ||
(scope && scope.has(alias));
alias = `${name}_${i++}`
);
this.used_names.add(alias);
Expand Down Expand Up @@ -707,8 +708,7 @@ export default class Component {
const remove = (parent, prop, index) => {
to_remove.unshift([parent, prop, index]);
};

const to_insert = new Map();
let scope_updated = false;

walk(content, {
enter(node, parent, prop, index) {
Expand All @@ -735,37 +735,21 @@ export default class Component {
}

component.warn_on_undefined_store_value_references(node, parent, scope);
},

leave(node) {
// do it on leave, to prevent infinite loop
if (component.compile_options.dev && component.compile_options.loopGuardTimeout > 0) {
const to_insert_for_loop_protect = component.loop_protect(node, prop, index, component.compile_options.loopGuardTimeout);
if (to_insert_for_loop_protect) {
if (!Array.isArray(parent[prop])) {
parent[prop] = {
type: 'BlockStatement',
body: [to_insert_for_loop_protect.node, node],
};
} else {
// can't insert directly, will screw up the index in the for-loop of estree-walker
if (!to_insert.has(parent)) {
to_insert.set(parent, []);
}
to_insert.get(parent).push(to_insert_for_loop_protect);
}
const to_replace_for_loop_protect = component.loop_protect(node, scope, component.compile_options.loopGuardTimeout);
if (to_replace_for_loop_protect) {
this.replace(to_replace_for_loop_protect);
scope_updated = true;
}
}
},

leave(node) {
if (map.has(node)) {
scope = scope.parent;
}
if (to_insert.has(node)) {
const nodes_to_insert = to_insert.get(node);
for (const { index, prop, node: node_to_insert } of nodes_to_insert.reverse()) {
node[prop].splice(index, 0, node_to_insert);
}
to_insert.delete(node);
}
}
},
});

Expand All @@ -778,6 +762,12 @@ export default class Component {
}
}
}

if (scope_updated) {
const { scope, map } = create_scopes(script.content);
this.instance_scope = scope;
this.instance_scope_map = map;
}
}

track_references_and_mutations() {
Expand Down Expand Up @@ -849,15 +839,12 @@ export default class Component {
}
}

loop_protect(node, prop, index, timeout) {
loop_protect(node, scope: Scope, timeout: number): Node | null {
if (node.type === 'WhileStatement' ||
node.type === 'ForStatement' ||
node.type === 'DoWhileStatement') {
const guard = this.get_unique_name('guard');
this.add_var({
name: guard.name,
internal: true,
});
const guard = this.get_unique_name('guard', scope);
this.used_names.add(guard.name);

const before = b`const ${guard} = @loop_guard(${timeout})`;
const inside = b`${guard}();`;
Expand All @@ -870,7 +857,14 @@ export default class Component {
};
}
node.body.body.push(inside[0]);
return { index, prop, node: before[0] };

return {
type: 'BlockStatement',
body: [
before[0],
node,
],
};
}
return null;
}
Expand Down
99 changes: 72 additions & 27 deletions test/js/samples/loop-protect/expected.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
/* generated by Svelte vX.Y.Z */
import {
SvelteComponentDev,
add_location,
binding_callbacks,
detach_dev,
dispatch_dev,
element,
init,
insert_dev,
loop_guard,
noop,
safe_not_equal
Expand All @@ -11,16 +16,27 @@ import {
const file = undefined;

function create_fragment(ctx) {
let div;

const block = {
c: noop,
c: function create() {
div = element("div");
add_location(div, file, 22, 0, 288);
},
l: function claim(nodes) {
throw new Error("options.hydrate only works if the component was compiled with the `hydratable: true` option");
},
m: noop,
m: function mount(target, anchor) {
insert_dev(target, div, anchor);
/*div_binding*/ ctx[1](div);
},
p: noop,
i: noop,
o: noop,
d: noop
d: function destroy(detaching) {
if (detaching) detach_dev(div);
/*div_binding*/ ctx[1](null);
}
};

dispatch_dev("SvelteRegisterBlock", {
Expand All @@ -34,62 +50,91 @@ function create_fragment(ctx) {
return block;
}

function instance($$self) {
const guard = loop_guard(100);
function foo() {
const guard = "foo";

{
const guard_1 = loop_guard(100);

while (true) {
foo();
guard();
while (true) {
console.log(guard);
guard_1();
}
}
}

function instance($$self, $$props, $$invalidate) {
let node;

const guard_1 = loop_guard(100);
{
const guard = loop_guard(100);

for (; ; ) {
foo();
guard_1();
while (true) {
foo();
guard();
}
}

const guard_2 = loop_guard(100);
{
const guard_2 = loop_guard(100);

while (true) {
foo();
guard_2();
for (; ; ) {
foo();
guard_2();
}
}

const guard_4 = loop_guard(100);
{
const guard_3 = loop_guard(100);

do {
foo();
guard_4();
} while (true);
while (true) {
foo();
guard_3();
}
}

{
const guard_5 = loop_guard(100);

do {
foo();
guard_5();
} while (true);
}

function div_binding($$value) {
binding_callbacks[$$value ? "unshift" : "push"](() => {
$$invalidate(0, node = $$value);
});
}

$$self.$capture_state = () => {
return {};
};

$$self.$inject_state = $$props => {

if ("node" in $$props) $$invalidate(0, node = $$props.node);
};

$: {
const guard_3 = loop_guard(100);
const guard_4 = loop_guard(100);

while (true) {
foo();
guard_3();
guard_4();
}
}

$: {
const guard_5 = loop_guard(100);
const guard_6 = loop_guard(100);

do {
foo();
guard_5();
guard_6();
} while (true);
}

return [];
return [node, div_binding];
}

class Component extends SvelteComponentDev {
Expand Down
13 changes: 12 additions & 1 deletion test/js/samples/loop-protect/input.svelte
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
<script>
let node;

function foo() {
const guard = 'foo';
while(true) {
console.log(guard);
}
}

while(true) {
foo();
}
Expand All @@ -9,4 +18,6 @@
$: while(true) foo();
do foo(); while(true);
$: do foo(); while(true);
</script>
</script>

<div bind:this={node}></div>
7 changes: 7 additions & 0 deletions test/runtime/samples/loop-protect-inner-function/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default {
html: '<div></div>',
compileOptions: {
dev: true,
loopGuardTimeout: 100,
}
};
7 changes: 7 additions & 0 deletions test/runtime/samples/loop-protect-inner-function/main.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script>
let node;
(function () {
while(false) ;
}());
</script>
<div bind:this={node}></div>