Skip to content
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

Explicit non-reentrancy invariant #38

Closed
guybedford opened this issue Apr 2, 2024 · 1 comment
Closed

Explicit non-reentrancy invariant #38

guybedford opened this issue Apr 2, 2024 · 1 comment

Comments

@guybedford
Copy link
Collaborator

guybedford commented Apr 2, 2024

There is a non-reentrancy requirement stated in Evaluate() as:

1. [Assert](https://tc39.es/ecma262/#assert): This call to Evaluate is not happening at the same time as another call to Evaluate within the [surrounding agent](https://tc39.es/ecma262/#surrounding-agent).

It is possible to violate this invariant with the current specification as written, by importing and evaluating a deferred module within the top-level evaluation of another module's execution.

For example:

main.mjs

import './dep.mjs';
console.log('main');

dep.mjs

import defer * as lazy from './lazy.mjs';
lazy.breakInvariant;
console.log('dep');

lazy.mjs

console.log('lazy');

The evaluation of main.mjs will result in lazy being executed between dep.mjs and main.mjs via a new top-level execution interleaved during the top-level execution of main, breaking the invariant of main being the only top-level execution.

We could:

  1. Relax the invariant to allow for disjoint graph evaluations to more carefully distinguish partial evaluating graphs, then only throwing specifically for the case where we hit modules already in the evaluating phase/
  2. Harden this invariant more explicitly by treating it as an explicit check with a real value.

I can try to work on a PR for a more relaxed invariant here if it would be useful.

This was referenced Apr 2, 2024
@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented May 31, 2024

Fixed by #39 doing (1).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants