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

Race condition in resolve #1334

Merged
merged 25 commits into from
Sep 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
ff339c1
feat: performance meter for actions
Anber Sep 5, 2023
a5edbb3
fix(babel): potential race condition in resolveImports
Anber Sep 5, 2023
b941800
chore: changeset
Anber Sep 5, 2023
f57376c
fix(babel): action recovery attempts happen on the wrong level
Anber Sep 7, 2023
ee02bc4
fix(babel): do not skip ignored files when explode reexports
Anber Sep 10, 2023
47c7145
fix(babel): removeDangerousCode was too agressive
Anber Sep 10, 2023
521d6c8
feat(babel): fileReporter for debug
Anber Sep 10, 2023
2438ed3
fix(babel): broken detection of identifier declaration
Anber Sep 11, 2023
2ee2b26
fix(babel): actions reruns return unexpected undefined instead of thr…
Anber Sep 11, 2023
9c09f84
fix(babel): actions' runner should process parallel throws
Anber Sep 11, 2023
8f55d77
fix(babel): add tests for async generators runner
Anber Sep 11, 2023
087de8c
fix(babel): do not skip ignored files in getExports
Anber Sep 11, 2023
f86ca3b
fix(babel): fix references detection for function expressions
Anber Sep 11, 2023
460b913
fix(babel): tests for createExports
Anber Sep 11, 2023
3297516
fix(testkit): fix failed tests
Anber Sep 11, 2023
42aabb4
fix(babel): fix for strange 'Cannot read properties of undefined' in …
Anber Sep 11, 2023
b585f99
fix(webpack): protection from fail-on-unhandled-errors
Anber Sep 11, 2023
5aa03cb
feat(babel): more logs for actions
Anber Sep 11, 2023
477c289
fix(babel): add logs for 'module is disposed' event
Anber Sep 12, 2023
4205d58
fix(babel): replace regex for esm detection
Anber Sep 12, 2023
2a51659
fix(babel): add log for resolved exports in getExports
Anber Sep 12, 2023
f5d121b
fix(babel): race condition on evalFile
Anber Sep 12, 2023
4476dd4
fix(babel): fix potential stack overflow in evalFIle
Anber Sep 14, 2023
fcace70
fix(babel): fix for module.exports=
Anber Sep 14, 2023
72d419d
fix(babel): fix for 'module is disposed'
Anber Sep 15, 2023
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
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
Loading