-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix(core): make daemon socket path unique per process to prevent race condition #33580
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Deploy Preview for nx-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
View your CI Pipeline Execution ↗ for commit c462b75
☁️ Nx Cloud last updated this comment at |
| await execAsync(preInstallCommand, execOptions); | ||
| } | ||
|
|
||
| await execAsync(installCommand, execOptions); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The execAsync (promisified exec) does not support the stdio option that exists in execOptions. The exec function only supports options like encoding, timeout, maxBuffer, etc., while stdio is only available for execSync and spawn. This will cause the stdio configuration to be silently ignored, potentially causing output to appear when it shouldn't or vice versa.
// Fix: Use separate options for async execution
const execAsyncOptions = {
cwd: tempDir,
windowsHide: false,
};
if (preInstallCommand) {
await execAsync(preInstallCommand, execAsyncOptions);
}
await execAsync(installCommand, execAsyncOptions);Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
🐳 We have a release for that!This PR has a release associated with it. You can try it out using this command: npx [email protected] my-workspaceOr just copy this version and use it in your own command: 0.0.0-pr-33580-5c615b0
To request a new release for this pull request, mention someone from the Nx team or the |
5c615b0 to
cf8fa1c
Compare
cf8fa1c to
47c95cd
Compare
| await this.queue.sendToQueue(async () => { | ||
| this.startDaemonIfNecessary(); | ||
|
|
||
| const socketPath = this.getSocketPath(); | ||
|
|
||
| messenger = new DaemonSocketMessenger(connect(socketPath)).listen( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical race condition: startDaemonIfNecessary() is async but not awaited. This will cause the code to proceed to get the socket path before the daemon has finished starting and writing the socket path to the cache file.
Fix:
await this.queue.sendToQueue(async () => {
await this.startDaemonIfNecessary();
const socketPath = this.getSocketPath();
messenger = new DaemonSocketMessenger(connect(socketPath)).listen(| await this.queue.sendToQueue(async () => { | |
| this.startDaemonIfNecessary(); | |
| const socketPath = this.getSocketPath(); | |
| messenger = new DaemonSocketMessenger(connect(socketPath)).listen( | |
| await this.queue.sendToQueue(async () => { | |
| await this.startDaemonIfNecessary(); | |
| const socketPath = this.getSocketPath(); | |
| messenger = new DaemonSocketMessenger(connect(socketPath)).listen( |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nx Cloud is proposing a fix for your failed CI:
These changes fix the daemon startup failures by resolving a circular dependency in isServerAvailable(). The method was calling getSocketPath() which required the daemon cache to exist, but during startup the daemon hadn't written its socket path yet, causing ENOENT errors. By having isServerAvailable() directly read from the cache and return false when no socket path exists, the daemon can now complete its startup sequence successfully.
diff --git a/packages/nx/src/daemon/client/client.ts b/packages/nx/src/daemon/client/client.ts
index 65f4013e96..2c3b1e4ed9 100644
--- a/packages/nx/src/daemon/client/client.ts
+++ b/packages/nx/src/daemon/client/client.ts
@@ -592,11 +592,19 @@ export class DaemonClient {
async isServerAvailable(): Promise<boolean> {
return new Promise((resolve) => {
try {
- const socketPath = this.getSocketPath();
- if (!socketPath) {
+ // Try to get socket path from cache first (for existing daemon)
+ const daemonProcessJson = readDaemonProcessJsonCache();
+ let socketPath: string;
+
+ if (daemonProcessJson?.socketPath) {
+ socketPath = daemonProcessJson.socketPath;
+ } else {
+ // Daemon hasn't started yet or no cache exists
+ // This can happen when checking availability during startup
resolve(false);
return;
}
+
const socket = connect(socketPath, () => {
socket.destroy();
resolve(true);
Or Apply changes locally with:
npx nx-cloud apply-locally fbRU-Bob2
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
12da26a to
32c7e25
Compare
9c1ce6b to
bdaa2fd
Compare
… condition Each daemon server now creates a socket path that includes its PID in the hash, preventing a race condition where an old daemon shutting down could remove a newly started daemon's socket file. Changes: - Include process.pid in socket directory hash - Store socket path in server-process.json - Client reads socket path from server-process.json - Ensure daemon is started if no socket path exists This prevents the cycle where daemons continuously start and shut down due to socket path conflicts.
bdaa2fd to
d1713d4
Compare
d1713d4 to
c462b75
Compare
| export function getInstalledNxVersion(): string | null { | ||
| try { | ||
| const nxPackageJsonPath = require.resolve('nx/package.json', { | ||
| paths: [workspaceRoot], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use require paths util



Current Behavior
Socket Race Condition: All daemon servers listen on the same socket path, causing a race condition where shutting down daemons remove sockets that newly started
daemons are listening on.
Daemon Console Check Blocks: The daemon availability check runs synchronously and blocks the main thread.
Version Mismatch Issues: Packages using a different nx version than what's installed in the workspace could still use the daemon, leading to potential issues.
Expected Behavior
Each daemon server creates a unique socket path based on its process ID, preventing race conditions.
The daemon console check runs in the background without blocking.
The daemon is disabled when there's a version mismatch between the running nx and the workspace's installed version.
Changes
1. Unique Daemon Socket Paths
process.pidin the socket directory hash to make each daemon's path uniqueserver-process.jsonso clients know where to connect2. Backgroundable Daemon Check
3. Version Mismatch Check
isNxVersionMismatch()check inDaemonClient.enabled()is-nx-version-mismatch.tsfor version comparisonrequire.resolve('nx/package.json', { paths: [workspaceRoot] })to properly resolve the workspace's installed nx versionRelated Issue(s)
Fixes daemon socket path race condition and improves daemon reliability.