fix #2396: propagate sync api errors from workers

CHANGELOG.md

@@ -6,6 +6,10 @@
 
     Previously esbuild would minify `0xFFFF_FFFF_FFFF_FFFF` as `0xffffffffffffffff` (18 bytes) on arm64 chips and as `18446744073709552e3` (19 bytes) on x86_64 chips. The reason was that the number was converted to a 64-bit unsigned integer internally for printing as hexadecimal, the 64-bit floating-point number `0xFFFF_FFFF_FFFF_FFFF` is actually `0x1_0000_0000_0000_0180` (i.e. it's rounded up, not down), and converting `float64` to `uint64` is implementation-dependent in Go when the input is out of bounds. This was fixed by changing the upper limit for which esbuild uses hexadecimal numbers during minification to `0xFFFF_FFFF_FFFF_F800`, which is the next representable 64-bit floating-point number below `0x1_0000_0000_0000_0180`, and which fits in a `uint64`. As a result, esbuild will now consistently never minify `0xFFFF_FFFF_FFFF_FFFF` as `0xffffffffffffffff` anymore, which means the output should now be consistent across platforms.
 
+* Fix a hang with the synchronous API when the package is corrupted ([#2396](https://github.com/evanw/esbuild/issues/2396))
+
+    An error message is already thrown when the esbuild package is corrupted and esbuild can't be run. However, if you are using a synchronous call in the JavaScript API in worker mode, esbuild will use a child worker to initialize esbuild once so that the overhead of initializing esbuild can be amortized across multiple synchronous API calls. However, errors thrown during initialization weren't being propagated correctly which resulted in a hang while the main thread waited forever for the child worker to finish initializing. With this release, initialization errors are now propagated correctly so calling a synchronous API call when the package is corrupted should now result in an error instead of a hang.
+
 ## 0.14.49
 
 * Keep inlined constants when direct `eval` is present ([#2361](https://github.com/evanw/esbuild/issues/2361))

lib/npm/node.ts

@@ -422,7 +422,6 @@ let startWorkerThreadService = (worker_threads: typeof import('worker_threads'))
     execArgv: [],
   });
   let nextID = 0;
-  let wasStopped = false;
 
   // This forbids options which would cause structured clone errors
   let fakeBuildError = (text: string) => {
@@ -452,7 +451,6 @@ let startWorkerThreadService = (worker_threads: typeof import('worker_threads'))
   };
 
   let runCallSync = (command: string, args: any[]): any => {
-    if (wasStopped) throw new Error('The service was stopped');
     let id = nextID++;
 
     // Make a fresh shared buffer for every request. That way we can't have a
@@ -509,12 +507,6 @@ let startWorkerThreadService = (worker_threads: typeof import('worker_threads'))
 let startSyncServiceWorker = () => {
   let workerPort: import('worker_threads').MessagePort = worker_threads!.workerData.workerPort;
   let parentPort = worker_threads!.parentPort!;
-  let service = ensureServiceIsRunning();
-
-  // Take the default working directory from the main thread because we want it
-  // to be consistent. This will be the working directory that was current at
-  // the time the "esbuild" package was first imported.
-  defaultWD = worker_threads!.workerData.defaultWD;
 
   // MessagePort doesn't copy the properties of Error objects. We still want
   // error objects to have extra properties such as "warnings" so implement the
@@ -529,35 +521,68 @@ let startSyncServiceWorker = () => {
     return properties;
   };
 
-  parentPort.on('message', (msg: MainToWorkerMessage) => {
-    (async () => {
-      let { sharedBuffer, id, command, args } = msg;
-      let sharedBufferView = new Int32Array(sharedBuffer);
+  try {
+    let service = ensureServiceIsRunning();
 
-      try {
-        switch (command) {
-          case 'build':
-            workerPort.postMessage({ id, resolve: await service.build(args[0]) });
-            break;
+    // Take the default working directory from the main thread because we want it
+    // to be consistent. This will be the working directory that was current at
+    // the time the "esbuild" package was first imported.
+    defaultWD = worker_threads!.workerData.defaultWD;
 
-          case 'transform':
-            workerPort.postMessage({ id, resolve: await service.transform(args[0], args[1]) });
-            break;
+    parentPort.on('message', (msg: MainToWorkerMessage) => {
+      (async () => {
+        let { sharedBuffer, id, command, args } = msg;
+        let sharedBufferView = new Int32Array(sharedBuffer);
 
-          case 'formatMessages':
-            workerPort.postMessage({ id, resolve: await service.formatMessages(args[0], args[1]) });
-            break;
+        try {
+          switch (command) {
+            case 'build':
+              workerPort.postMessage({ id, resolve: await service.build(args[0]) });
+              break;
+
+            case 'transform':
+              workerPort.postMessage({ id, resolve: await service.transform(args[0], args[1]) });
+              break;
+
+            case 'formatMessages':
+              workerPort.postMessage({ id, resolve: await service.formatMessages(args[0], args[1]) });
+              break;
+
+            case 'analyzeMetafile':
+              workerPort.postMessage({ id, resolve: await service.analyzeMetafile(args[0], args[1]) });
+              break;
+
+            default:
+              throw new Error(`Invalid command: ${command}`);
+          }
+        } catch (reject) {
+          workerPort.postMessage({ id, reject, properties: extractProperties(reject) });
+        }
 
-          case 'analyzeMetafile':
-            workerPort.postMessage({ id, resolve: await service.analyzeMetafile(args[0], args[1]) });
-            break;
+        // The message has already been posted by this point, so it should be
+        // safe to wake the main thread. The main thread should always get the
+        // message we sent above.
 
-          default:
-            throw new Error(`Invalid command: ${command}`);
-        }
-      } catch (reject) {
-        workerPort.postMessage({ id, reject, properties: extractProperties(reject) });
-      }
+        // First, change the shared value. That way if the main thread attempts
+        // to wait for us after this point, the wait will fail because the shared
+        // value has changed.
+        Atomics.add(sharedBufferView, 0, 1);
+
+        // Then, wake the main thread. This handles the case where the main
+        // thread was already waiting for us before the shared value was changed.
+        Atomics.notify(sharedBufferView, 0, Infinity);
+      })();
+    });
+  }
+
+  // Creating the service can fail if the on-disk state is corrupt. In that case
+  // we just fail all incoming messages with whatever error message we got.
+  // Otherwise incoming messages will hang forever waiting for a reply.
+  catch (reject) {
+    parentPort.on('message', (msg: MainToWorkerMessage) => {
+      let { sharedBuffer, id } = msg;
+      let sharedBufferView = new Int32Array(sharedBuffer);
+      workerPort.postMessage({ id, reject, properties: extractProperties(reject) });
 
       // The message has already been posted by this point, so it should be
       // safe to wake the main thread. The main thread should always get the
@@ -571,8 +596,8 @@ let startSyncServiceWorker = () => {
       // Then, wake the main thread. This handles the case where the main
       // thread was already waiting for us before the shared value was changed.
       Atomics.notify(sharedBufferView, 0, Infinity);
-    })();
-  });
+    });
+  }
 };
 
 // If we're in the worker thread, start the worker code