Skip to content

chore: provisional esm deadlock fix #36143

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

Closed
wants to merge 2 commits into from
Closed
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
29 changes: 3 additions & 26 deletions packages/playwright/src/transform/esmLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,10 @@
import fs from 'fs';
import url from 'url';

import { monotonicTime, raceAgainstDeadline } from 'playwright-core/lib/utils';

import { addToCompilationCache, currentFileDepsCollector, serializeCompilationCache, startCollectingFileDeps, stopCollectingFileDeps } from './compilationCache';
import { PortTransport } from './portTransport';
import { resolveHook, setSingleTSConfig, setTransformConfig, shouldTransform, transformHook } from './transform';
import { debugTest, fileIsModule } from '../util';

// See note on pushToCompilationCache()
// Once we enter a deadlock scenario, we will fallback to unawaited IPC
let workerShouldFallbackCompilationCache = false;
import { fileIsModule } from '../util';

// Node < 18.6: defaultResolve takes 3 arguments.
// Node >= 18.6: nextResolve from the chain takes 2 arguments.
Expand Down Expand Up @@ -60,7 +54,7 @@ const kSupportedFormats = new Map([

// Node < 18.6: defaultLoad takes 3 arguments.
// Node >= 18.6: nextLoad from the chain takes 2 arguments.
async function load(moduleUrl: string, context: { format?: string }, defaultLoad: Function) {
function load(moduleUrl: string, context: { format?: string }, defaultLoad: Function) {
// Bail out for wasm, json, etc.
if (!kSupportedFormats.has(context.format))
return defaultLoad(moduleUrl, context, defaultLoad);
Expand All @@ -79,7 +73,7 @@ async function load(moduleUrl: string, context: { format?: string }, defaultLoad

// Flush the source maps to the main thread, so that errors during import() are source-mapped.
if (transformed.serializedCache && transport)
await pushToCompilationCache(transport, transformed.serializedCache);
transport.post('pushToCompilationCache', { cache: transformed.serializedCache });

// Output format is required, so we determine it manually when unknown.
// shortCircuit is required by Node >= 18.6 to designate no more loaders should be called.
Expand All @@ -90,23 +84,6 @@ async function load(moduleUrl: string, context: { format?: string }, defaultLoad
};
}

// Under certain conditions with ESM -> CJS -> any imports, we can enter deadlock awaiting the
// MessagePort transfer simultaneously with the Node.js worker thread that is performing the load().
// Attempt to await the IPC transfer, and if it takes too long, fallback to a non-awaiting transfer
async function pushToCompilationCache(transport: PortTransport, cache: any) {
if (workerShouldFallbackCompilationCache) {
transport.send('pushToCompilationCache', { cache })
.catch(e => debugTest('Failed to push compilation cache', e));
return;
}

const { timedOut } = await raceAgainstDeadline(() => transport.send('pushToCompilationCache', { cache }), monotonicTime() + 1000);
if (timedOut) {
debugTest('Falling back to unawaited compilation cache');
workerShouldFallbackCompilationCache = true;
}
}

let transport: PortTransport | undefined;

function initialize(data: { port: MessagePort }) {
Expand Down
14 changes: 8 additions & 6 deletions packages/playwright/src/transform/portTransport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,27 @@ export class PortTransport {
port.addEventListener('message', async event => {
const message = event.data;
const { id, ackId, method, params, result } = message;
if (id) {
const result = await handler(method, params);
this._port.postMessage({ ackId: id, result });
return;
}

if (ackId) {
const callback = this._callbacks.get(ackId);
this._callbacks.delete(ackId);
this._resetRef();
callback?.(result);
return;
}

const handlerResult = await handler(method, params);
if (id)
this._port.postMessage({ ackId: id, result: handlerResult });
});
// Make sure to unref **after** adding a 'message' event listener.
// https://nodejs.org/api/worker_threads.html#portref
this._resetRef();
}

post(method: string, params: any) {
this._port.postMessage({ method, params });
}

async send(method: string, params: any) {
return await new Promise<any>(f => {
const id = ++this._lastId;
Expand Down
11 changes: 9 additions & 2 deletions packages/playwright/src/transform/transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,15 @@ export async function requireOrImport(file: string) {
installTransformIfNeeded();
const isModule = fileIsModule(file);
const esmImport = () => eval(`import(${JSON.stringify(url.pathToFileURL(file))})`);
if (isModule)
return await esmImport();
if (isModule) {
return await esmImport().finally(async () => {
// Compilation cache, which includes source maps, is populated in a post task.
// When importing a module results in an error, the very next access to `error.stack` will need source maps.
// To make sure source maps have arrived, we insert a task that will be processed after
// compilation cache and guarantee that source maps are available, before `error.stack` is accessed.
await new Promise(resolve => setTimeout(resolve, 0));
});
}
const result = require(file);
const depsCollector = currentFileDepsCollector();
if (depsCollector) {
Expand Down
Loading