Skip to content

Commit

Permalink
Race condition in resolve (#1334)
Browse files Browse the repository at this point in the history
* feat: performance meter for actions

* fix(babel): potential race condition in resolveImports

* chore: changeset

* fix(babel): action recovery attempts happen on the wrong level

* fix(babel): do not skip ignored files when explode reexports

* fix(babel): removeDangerousCode was too agressive

* feat(babel): fileReporter for debug

* fix(babel): broken detection of identifier declaration

* fix(babel): actions reruns return unexpected undefined instead of throwing error when first run fails

* fix(babel): actions' runner should process parallel throws

* fix(babel): add tests for async generators runner

* fix(babel): do not skip ignored files in getExports

* fix(babel): fix references detection for function expressions

* fix(babel): tests for createExports

* fix(testkit): fix failed tests

* fix(babel): fix for strange 'Cannot read properties of undefined' in tests

* fix(webpack): protection from fail-on-unhandled-errors

* feat(babel): more logs for actions

* fix(babel): add logs for 'module is disposed' event

* fix(babel): replace regex for esm detection

* fix(babel): add log for resolved exports in getExports

* fix(babel): race condition on evalFile

* fix(babel): fix potential stack overflow in evalFIle

* fix(babel): fix for module.exports=

* fix(babel): fix for 'module is disposed'
  • Loading branch information
Anber authored Sep 15, 2023
1 parent 911b59b commit aa10045
Show file tree
Hide file tree
Showing 64 changed files with 1,661 additions and 656 deletions.
7 changes: 7 additions & 0 deletions .changeset/pink-moose-cheer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@linaria/babel-preset': patch
'@linaria/testkit': patch
'@linaria/utils': patch
---

In some cases, an asynchronous resolver could cause race conditions. Fixed.
77 changes: 23 additions & 54 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,67 +59,36 @@ const memberOrder = [

'constructor',

// Getters
'public-static-get',
'protected-static-get',
'private-static-get',
'#private-static-get',
// Getters & Setters
['public-static-get', 'public-static-set'],
['protected-static-get', 'protected-static-set'],
['private-static-get', 'private-static-set'],
['#private-static-get', '#private-static-set'],

'public-decorated-get',
'protected-decorated-get',
'private-decorated-get',
['public-decorated-get', 'public-decorated-set'],
['protected-decorated-get', 'protected-decorated-set'],
['private-decorated-get', 'private-decorated-set'],

'public-instance-get',
'protected-instance-get',
'private-instance-get',
'#private-instance-get',
['public-instance-get', 'public-instance-set'],
['protected-instance-get', 'protected-instance-set'],
['private-instance-get', 'private-instance-set'],
['#private-instance-get', '#private-instance-set'],

'public-abstract-get',
'protected-abstract-get',
['public-abstract-get', 'public-abstract-set'],
['protected-abstract-get', 'protected-abstract-set'],

'public-get',
'protected-get',
'private-get',
'#private-get',
['public-get', 'public-set'],
['protected-get', 'protected-set'],
['private-get', 'private-set'],
['#private-get', '#private-set'],

'static-get',
'instance-get',
'abstract-get',
['static-get', 'static-set'],
['instance-get', 'instance-set'],
['abstract-get', 'abstract-set'],

'decorated-get',
['decorated-get', 'decorated-set'],

'get',

// Setters
'public-static-set',
'protected-static-set',
'private-static-set',
'#private-static-set',

'public-decorated-set',
'protected-decorated-set',
'private-decorated-set',

'public-instance-set',
'protected-instance-set',
'private-instance-set',
'#private-instance-set',

'public-abstract-set',
'protected-abstract-set',

'public-set',
'protected-set',
'private-set',
'#private-set',

'static-set',
'instance-set',
'abstract-set',

'decorated-set',

'set',
['get', 'set'],

// Methods
'public-static-method',
Expand Down
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ logs
npm-debug.log*
yarn-debug.log*
yarn-error.log*
**/linaria-debug.json
**/linaria-debug

# Runtime data
pids
Expand Down
2 changes: 1 addition & 1 deletion docs/CONFIGURATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ module.exports = {
return false;
}

return /\b(?:export|import)\b/m.test(code);
return /(?:^|\*\/|;)\s*(?:export|import)\s/m.test(code);
},
action: require.resolve('@linaria/shaker'),
}
Expand Down
2 changes: 1 addition & 1 deletion examples/vite/.linariarc.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ module.exports = {
return false;
}

return /(?:^|\n|;)\s*(?:export|import)\s+/m.test(code);
return /(?:^|\*\/|;)\s*(?:export|import)\s/m.test(code);
},
action: require.resolve('@linaria/shaker'),
},
Expand Down
8 changes: 8 additions & 0 deletions packages/babel/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,12 @@ module.exports = {
preset: 'ts-jest',
testEnvironment: 'node',
testMatch: ['**/__tests__/**/*.test.ts'],
transform: {
'^.+\\.ts$': [
'ts-jest',
{
tsconfig: 'tsconfig.spec.json',
},
],
},
};
3 changes: 1 addition & 2 deletions packages/babel/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,9 @@
"@types/babel__helper-module-imports": "^7.18.0",
"@types/babel__template": "^7.4.1",
"@types/babel__traverse": "^7.20.1",
"@types/dedent": "^0.7.0",
"@types/jest": "^28.1.0",
"@types/node": "^17.0.39",
"dedent": "^0.7.0",
"dedent": "^1.5.1",
"jest": "^29.6.2",
"strip-ansi": "^5.2.0",
"ts-jest": "^29.1.1",
Expand Down
9 changes: 7 additions & 2 deletions packages/babel/src/evaluators/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,16 @@ import type { TransformCacheCollection } from '../cache';
import Module from '../module';
import type { Entrypoint } from '../transform/Entrypoint';

export interface IEvaluateResult {
dependencies: string[];
value: Record<string | symbol, unknown>;
}

export default function evaluate(
cache: TransformCacheCollection,
entrypoint: Entrypoint
) {
using m = new Module(entrypoint, cache);
): IEvaluateResult {
const m = new Module(entrypoint, cache);

m.evaluate();

Expand Down
4 changes: 4 additions & 0 deletions packages/babel/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ export {
} from './utils/withLinariaMetadata';
export { default as Module, DefaultModuleImplementation } from './module';
export { default as transform } from './transform';
export {
isUnprocessedEntrypointError,
UnprocessedEntrypointError,
} from './transform/actions/UnprocessedEntrypointError';
export * from './types';
export { EvaluatedEntrypoint } from './transform/EvaluatedEntrypoint';
export type { IEvaluatedEntrypoint } from './transform/EvaluatedEntrypoint';
Expand Down
77 changes: 29 additions & 48 deletions packages/babel/src/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ import { Entrypoint } from './transform/Entrypoint';
import { getStack, isSuperSet } from './transform/Entrypoint.helpers';
import type { IEntrypointDependency } from './transform/Entrypoint.types';
import type { IEvaluatedEntrypoint } from './transform/EvaluatedEntrypoint';
import { syncActionRunner } from './transform/actions/actionRunner';
import { baseProcessingHandlers } from './transform/generators/baseProcessingHandlers';
import { syncResolveImports } from './transform/generators/resolveImports';
import { isUnprocessedEntrypointError } from './transform/actions/UnprocessedEntrypointError';
import loadLinariaOptions from './transform/helpers/loadLinariaOptions';
import { withDefaultServices } from './transform/helpers/withDefaultServices';
import { createVmContext } from './vm/createVmContext';
Expand Down Expand Up @@ -106,13 +104,7 @@ function resolve(
return resolved;
}

function assertDisposed(
entrypoint: Entrypoint | null
): asserts entrypoint is Entrypoint {
invariant(entrypoint, 'Module is disposed');
}

class Module implements Disposable {
class Module {
public readonly callstack: string[] = [];

public readonly debug: Debugger;
Expand Down Expand Up @@ -186,7 +178,7 @@ class Module implements Disposable {
return entrypoint.exports;
}

using m = new Module(entrypoint, this.cache, this);
const m = this.createChild(entrypoint);
m.evaluate();

return entrypoint.exports;
Expand All @@ -199,15 +191,15 @@ class Module implements Disposable {

public resolve = resolve.bind(this);

protected entrypoint: Entrypoint | null;
#entrypointRef: WeakRef<Entrypoint>;

constructor(
entrypoint: Entrypoint,
private cache = new TransformCacheCollection(),
parentModule?: Module,
private moduleImpl: HiddenModuleMembers = DefaultModuleImplementation
) {
this.entrypoint = entrypoint;
this.#entrypointRef = new WeakRef(entrypoint);
this.idx = entrypoint.idx;
this.id = entrypoint.name;
this.filename = entrypoint.name;
Expand All @@ -228,42 +220,37 @@ class Module implements Disposable {
}

public get exports() {
assertDisposed(this.entrypoint);
return this.entrypoint.exports;
}

public set exports(value) {
assertDisposed(this.entrypoint);

this.entrypoint.exports = value;

this.debug('the whole exports was overridden with %O', value);
}

[Symbol.dispose](): void {
assertDisposed(this.entrypoint);

this.debug('dispose');

this.entrypoint = null;
protected get entrypoint(): Entrypoint {
const entrypoint = this.#entrypointRef.deref();
invariant(entrypoint, `Module ${this.idx} is disposed`);
return entrypoint;
}

evaluate(): void {
assertDisposed(this.entrypoint);

const { entrypoint } = this;
entrypoint.assertTransformed();

const cached = this.cache.get('entrypoints', entrypoint.name)!;
let evaluatedCreated = false;
if (!entrypoint.supersededWith) {
this.cache.add(
'entrypoints',
entrypoint.name,
entrypoint.createEvaluated()
);
evaluatedCreated = true;
}

const source =
entrypoint.transformedCode ??
entrypoint.originalCode ??
entrypoint.initialCode;
const source = entrypoint.transformedCode;

if (!source) {
this.debug(`evaluate`, 'there is nothing to evaluate');
Expand Down Expand Up @@ -310,6 +297,16 @@ class Module implements Disposable {

script.runInContext(context);
} catch (e) {
this.isEvaluated = false;
if (evaluatedCreated) {
this.cache.add('entrypoints', entrypoint.name, cached);
}

if (isUnprocessedEntrypointError(e)) {
// It will be handled by evalFile scenario
throw e;
}

if (e instanceof EvalError) {
this.debug('%O', e);

Expand All @@ -330,8 +327,6 @@ class Module implements Disposable {
only: string[],
log: Debugger
): Entrypoint | IEvaluatedEntrypoint | null {
assertDisposed(this.entrypoint);

const extension = path.extname(filename);
if (extension !== '.json' && !this.extensions.includes(extension)) {
return null;
Expand Down Expand Up @@ -397,14 +392,6 @@ class Module implements Disposable {
},
});

const syncResolve = (what: string, importer: string): string => {
return this.moduleImpl._resolveFilename(what, {
id: importer,
filename: importer,
paths: this.moduleImpl._nodeModulePaths(path.dirname(importer)),
});
};

const pluginOptions = loadLinariaOptions({});
const code = fs.readFileSync(filename, 'utf-8');
const newEntrypoint = Entrypoint.createRoot(
Expand All @@ -427,20 +414,10 @@ class Module implements Disposable {
return newEntrypoint;
}

const action = newEntrypoint.createAction('processEntrypoint', undefined);
syncActionRunner(action, {
...baseProcessingHandlers,
resolveImports() {
return syncResolveImports.call(this, syncResolve);
},
});

return newEntrypoint;
}

resolveDependency = (id: string): IEntrypointDependency => {
assertDisposed(this.entrypoint);

const cached = this.entrypoint.getDependency(id);
invariant(!(cached instanceof Promise), 'Dependency is not resolved yet');

Expand Down Expand Up @@ -489,6 +466,10 @@ class Module implements Disposable {
added.forEach((ext) => delete extensions[ext]);
}
};

protected createChild(entrypoint: Entrypoint): Module {
return new Module(entrypoint, this.cache, this, this.moduleImpl);
}
}

export default Module;
Loading

0 comments on commit aa10045

Please sign in to comment.