Skip to content

Race condition in daemon PID file management #7

Description

@AgentWOPR

Bug Description:
In src/cli.ts lines 45-55, there's a race condition between checking if a process exists and cleaning up the PID file. Multiple processes could simultaneously detect a stale PID and try to clean it up.

Location:

  • File: src/cli.ts
  • Lines: 45-55

Code:

function getDaemonPid(): number | null {
  if (\!existsSync(PID_FILE)) return null;
  const pid = parseInt(readFileSync(PID_FILE, "utf-8").trim());
  try {
    process.kill(pid, 0);
    return pid;
  } catch {
    unlinkSync(PID_FILE);  // Race condition here
    return null;
  }
}

Issues:

  1. Race Condition: Multiple processes can try to unlinkSync(PID_FILE) simultaneously
  2. TOCTOU Bug: File could be deleted between existsSync and readFileSync
  3. Error Handling: parseInt could return NaN for corrupted PID files
  4. Signal 0 Limitation: process.kill(pid, 0) doesn't work on Windows

Potential Scenarios:

  • Two CLI calls happen simultaneously with stale PID file
  • One process deletes PID file while another tries to read it
  • Corrupted PID file causes parseInt(NaN) to be passed to process.kill

Recommended Fixes:

  1. Add proper error handling for file operations
  2. Validate PID before using it
  3. Use atomic operations or file locking
  4. Handle Windows compatibility

Severity: Low-Medium - Could cause sporadic failures in daemon management

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions