-
Notifications
You must be signed in to change notification settings - Fork 32
Description
sass/dart-sass#2312 made embedded compiler shutdown upon ProtocolError significantly faster, which revealed a race condition in SyncMessagePort that caused flaky test failures: https://github.com/sass/dart-sass/actions/runs/10552748019/job/29232018679
embedded-host-node/lib/src/sync-process/sync-message-port.ts
Lines 127 to 159 in b545759
| receiveMessage(): unknown { | |
| if (this.listenerCount('message')) { | |
| throw new Error( | |
| 'SyncMessageChannel.receiveMessage() may not be called while there ' + | |
| 'are message listeners.' | |
| ); | |
| } | |
| // Set the "new message" indicator to zero before we check for new messages. | |
| // That way if the other port sets it to 1 between the call to | |
| // `receiveMessageOnPort` and the call to `Atomics.wait()`, we won't | |
| // overwrite it. Use `Atomics.compareExchange` so that we don't overwrite | |
| // the "closed" state. | |
| if ( | |
| Atomics.compareExchange( | |
| this.buffer, | |
| 0, | |
| BufferState.MessageSent, | |
| BufferState.AwaitingMessage | |
| ) === BufferState.Closed | |
| ) { | |
| throw new Error("The SyncMessagePort's channel is closed."); | |
| } | |
| let message = receiveMessageOnPort(this.port); | |
| if (message) return message.message; | |
| // If there's no new message, wait for the other port to flip the "new | |
| // message" indicator to 1. If it's been set to 1 since we stored 0, this | |
| // will terminate immediately. | |
| Atomics.wait(this.buffer, 0, BufferState.AwaitingMessage); | |
| message = receiveMessageOnPort(this.port); | |
| if (message) return message.message; |
During shutdown the worker would post a {"type":"exit","code":76} event. For most of the cases, the main thread is fast enough that it's already passed the check of Atomics.compareExchange(this.buffer, 0, BufferState.MessageSent, BufferState.AwaitingMessage) === BufferState.Closed, and already waiting at Atomics.wait(this.buffer, 0, BufferState.AwaitingMessage). In such case, the exit event is consumed correctly.
However, when the main thread is running slow, the port may have been closed before the check of Atomics.compareExchange(this.buffer, 0, BufferState.MessageSent, BufferState.AwaitingMessage) === BufferState.Closed, and by the time it runs the check, the error the SyncMessagePort's channel is closed is thrown, with the last exit event message not able to be consumed.