Skip to content
Draft
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
17 changes: 17 additions & 0 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { customToolRegistry } from "@roo-code/core"
import "./utils/path" // Necessary to have access to String.prototype.toPosix.
import { createOutputChannelLogger, createDualLogger } from "./utils/outputChannelLogger"
import { initializeNetworkProxy } from "./utils/networkProxy"
import { cleanupStaleLocks } from "./utils/safeWriteJson"

import { Package } from "./shared/package"
import { formatLanguage } from "./shared/language"
Expand Down Expand Up @@ -77,6 +78,22 @@ export async function activate(context: vscode.ExtensionContext) {
// Set extension path for custom tool registry to find bundled esbuild
customToolRegistry.setExtensionPath(context.extensionPath)

// Clean up stale lock files early, before any file operations that use safeWriteJson.
// This prevents hangs on btrfs and other filesystems where lock detection may not
// work correctly after extension updates or crashes. See issue #10436.
try {
const globalStoragePath = context.globalStorageUri.fsPath
const removedCount = await cleanupStaleLocks(globalStoragePath)
if (removedCount > 0) {
outputChannel.appendLine(`Cleaned up ${removedCount} stale lock file(s) from previous session`)
}
} catch (error) {
// Don't block activation on cleanup failures
outputChannel.appendLine(
`Warning: Failed to cleanup stale locks: ${error instanceof Error ? error.message : String(error)}`,
)
}

// Migrate old settings to new
await migrateSettings(context, outputChannel)

Expand Down
218 changes: 217 additions & 1 deletion src/utils/__tests__/safeWriteJson.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Writable } from "stream"
import * as path from "path"
import * as os from "os"

import { safeWriteJson } from "../safeWriteJson"
import { safeWriteJson, cleanupStaleLocks, LOCK_STALE_MS } from "../safeWriteJson"

const originalFsPromisesRename = actualFsPromises.rename
const originalFsPromisesUnlink = actualFsPromises.unlink
Expand Down Expand Up @@ -478,3 +478,219 @@ describe("safeWriteJson", () => {
consoleErrorSpy.mockRestore()
})
})

describe("cleanupStaleLocks", () => {
let tempDir: string

beforeEach(async () => {
// Create a temporary directory for each test
tempDir = await actualFsPromises.mkdtemp(path.join(os.tmpdir(), "cleanupStaleLocks-test-"))
})

afterEach(async () => {
// Clean up the temporary directory after each test
await actualFsPromises.rm(tempDir, { recursive: true, force: true })
})

test("should return 0 when directory does not exist", async () => {
const nonExistentPath = path.join(tempDir, "non-existent")
const result = await cleanupStaleLocks(nonExistentPath)
expect(result).toBe(0)
})

test("should return 0 when directory is empty", async () => {
const result = await cleanupStaleLocks(tempDir)
expect(result).toBe(0)
})

test("should return 0 when no lock files exist", async () => {
// Create some regular files (not locks)
await actualFsPromises.writeFile(path.join(tempDir, "file1.json"), "{}")
await actualFsPromises.writeFile(path.join(tempDir, "file2.txt"), "test")

const result = await cleanupStaleLocks(tempDir)
expect(result).toBe(0)

// Verify regular files still exist
await expect(actualFsPromises.access(path.join(tempDir, "file1.json"))).resolves.toBeUndefined()
await expect(actualFsPromises.access(path.join(tempDir, "file2.txt"))).resolves.toBeUndefined()
})

test("should remove stale .lock directory", async () => {
// Create a stale lock directory
const lockPath = path.join(tempDir, "file.json.lock")
await actualFsPromises.mkdir(lockPath)

// Set mtime to be older than LOCK_STALE_MS
const staleTime = new Date(Date.now() - LOCK_STALE_MS - 5000)
await actualFsPromises.utimes(lockPath, staleTime, staleTime)

const result = await cleanupStaleLocks(tempDir)
expect(result).toBe(1)

// Verify lock directory was removed
await expect(actualFsPromises.access(lockPath)).rejects.toThrow()
})

test("should remove stale .lock file", async () => {
// Create a stale lock file
const lockPath = path.join(tempDir, "file.json.lock")
await actualFsPromises.writeFile(lockPath, "")

// Set mtime to be older than LOCK_STALE_MS
const staleTime = new Date(Date.now() - LOCK_STALE_MS - 5000)
await actualFsPromises.utimes(lockPath, staleTime, staleTime)

const result = await cleanupStaleLocks(tempDir)
expect(result).toBe(1)

// Verify lock file was removed
await expect(actualFsPromises.access(lockPath)).rejects.toThrow()
})

test("should NOT remove recent .lock directory", async () => {
// Create a recent lock directory (not stale)
const lockPath = path.join(tempDir, "file.json.lock")
await actualFsPromises.mkdir(lockPath)

// mtime will be recent (now)

const result = await cleanupStaleLocks(tempDir)
expect(result).toBe(0)

// Verify lock directory still exists
await expect(actualFsPromises.access(lockPath)).resolves.toBeUndefined()
})

test("should remove multiple stale locks", async () => {
// Create multiple stale lock directories
const lock1 = path.join(tempDir, "file1.json.lock")
const lock2 = path.join(tempDir, "file2.json.lock")
const lock3 = path.join(tempDir, "settings.lock")

await actualFsPromises.mkdir(lock1)
await actualFsPromises.mkdir(lock2)
await actualFsPromises.writeFile(lock3, "")

// Set mtime to be older than LOCK_STALE_MS
const staleTime = new Date(Date.now() - LOCK_STALE_MS - 5000)
await actualFsPromises.utimes(lock1, staleTime, staleTime)
await actualFsPromises.utimes(lock2, staleTime, staleTime)
await actualFsPromises.utimes(lock3, staleTime, staleTime)

const result = await cleanupStaleLocks(tempDir)
expect(result).toBe(3)

// Verify all locks were removed
await expect(actualFsPromises.access(lock1)).rejects.toThrow()
await expect(actualFsPromises.access(lock2)).rejects.toThrow()
await expect(actualFsPromises.access(lock3)).rejects.toThrow()
})

test("should recursively clean subdirectories by default", async () => {
// Create nested directory structure with locks
const subDir = path.join(tempDir, "subdir")
await actualFsPromises.mkdir(subDir)

const lock1 = path.join(tempDir, "file1.json.lock")
const lock2 = path.join(subDir, "file2.json.lock")

await actualFsPromises.mkdir(lock1)
await actualFsPromises.mkdir(lock2)

// Set mtime to be older than LOCK_STALE_MS
const staleTime = new Date(Date.now() - LOCK_STALE_MS - 5000)
await actualFsPromises.utimes(lock1, staleTime, staleTime)
await actualFsPromises.utimes(lock2, staleTime, staleTime)

const result = await cleanupStaleLocks(tempDir, { recursive: true })
expect(result).toBe(2)

// Verify both locks were removed
await expect(actualFsPromises.access(lock1)).rejects.toThrow()
await expect(actualFsPromises.access(lock2)).rejects.toThrow()

// Verify subdir still exists (only locks removed)
await expect(actualFsPromises.access(subDir)).resolves.toBeUndefined()
})

test("should not recursively clean when recursive is false", async () => {
// Create nested directory structure with locks
const subDir = path.join(tempDir, "subdir")
await actualFsPromises.mkdir(subDir)

const lock1 = path.join(tempDir, "file1.json.lock")
const lock2 = path.join(subDir, "file2.json.lock")

await actualFsPromises.mkdir(lock1)
await actualFsPromises.mkdir(lock2)

// Set mtime to be older than LOCK_STALE_MS
const staleTime = new Date(Date.now() - LOCK_STALE_MS - 5000)
await actualFsPromises.utimes(lock1, staleTime, staleTime)
await actualFsPromises.utimes(lock2, staleTime, staleTime)

const result = await cleanupStaleLocks(tempDir, { recursive: false })
expect(result).toBe(1)

// Verify only top-level lock was removed
await expect(actualFsPromises.access(lock1)).rejects.toThrow()

// Lock in subdir should still exist
await expect(actualFsPromises.access(lock2)).resolves.toBeUndefined()
})

test("should use custom staleDurationMs when provided", async () => {
// Create a lock directory
const lockPath = path.join(tempDir, "file.json.lock")
await actualFsPromises.mkdir(lockPath)

// Set mtime to 10 seconds ago
const tenSecondsAgo = new Date(Date.now() - 10000)
await actualFsPromises.utimes(lockPath, tenSecondsAgo, tenSecondsAgo)

// Should not remove with default stale duration (31s)
const result1 = await cleanupStaleLocks(tempDir)
expect(result1).toBe(0)

// Should remove with custom stale duration (5s)
const result2 = await cleanupStaleLocks(tempDir, { staleDurationMs: 5000 })
expect(result2).toBe(1)

// Verify lock was removed
await expect(actualFsPromises.access(lockPath)).rejects.toThrow()
})

test("should handle errors gracefully and continue", async () => {
// Create a stale lock directory
const lockPath = path.join(tempDir, "file.json.lock")
await actualFsPromises.mkdir(lockPath)

// Set mtime to be older than LOCK_STALE_MS
const staleTime = new Date(Date.now() - LOCK_STALE_MS - 5000)
await actualFsPromises.utimes(lockPath, staleTime, staleTime)

// Suppress console output during this test
const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {})
const consoleLogSpy = vi.spyOn(console, "log").mockImplementation(() => {})

// Mock rm to fail for this specific lock
const rmSpy = vi.spyOn(actualFsPromises, "rm").mockRejectedValueOnce(new Error("Permission denied"))

const result = await cleanupStaleLocks(tempDir)

// Should return 0 because removal failed
expect(result).toBe(0)

// Verify console.warn was called
expect(consoleWarnSpy).toHaveBeenCalled()

consoleWarnSpy.mockRestore()
consoleLogSpy.mockRestore()
rmSpy.mockRestore()
})

test("should export LOCK_STALE_MS constant", () => {
expect(LOCK_STALE_MS).toBe(31000)
})
})
82 changes: 81 additions & 1 deletion src/utils/safeWriteJson.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ import * as lockfile from "proper-lockfile"
import Disassembler from "stream-json/Disassembler"
import Stringer from "stream-json/Stringer"

/**
* The stale timeout for locks in milliseconds (must match the value in safeWriteJson).
* Locks older than this are considered abandoned and can be safely removed.
*/
const LOCK_STALE_MS = 31000
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The safeWriteJson function still uses a hardcoded 31000 value for the stale option (line 49). Consider updating it to use LOCK_STALE_MS to keep both values synchronized and avoid potential inconsistencies if one is changed without the other.

Suggested change
const LOCK_STALE_MS = 31000
const LOCK_STALE_MS = 31_000

Fix it with Roo Code or mention @roomote and request a fix.


/**
* Safely writes JSON data to a file.
* - Creates parent directories if they don't exist
Expand Down Expand Up @@ -232,4 +238,78 @@ async function _streamDataToFile(targetPath: string, data: any): Promise<void> {
})
}

export { safeWriteJson }
/**
* Cleans up stale lock files/directories from a directory.
*
* The `proper-lockfile` library creates `.lock` directories for advisory locking.
* On some filesystems (particularly btrfs with Copy-on-Write), these locks may not
* be properly detected as stale, causing hangs during extension activation after
* updates or crashes.
*
* This function scans a directory for `.lock` entries and removes any that are
* older than the stale timeout, ensuring clean startup.
*
* @param directoryPath - The directory to scan for stale locks
* @param options - Optional configuration
* @param options.recursive - If true, scan subdirectories as well (default: true)
* @param options.staleDurationMs - Custom stale duration in ms (default: LOCK_STALE_MS)
* @returns Promise<number> - Number of stale locks removed
*/
async function cleanupStaleLocks(
directoryPath: string,
options: { recursive?: boolean; staleDurationMs?: number } = {},
): Promise<number> {
const { recursive = true, staleDurationMs = LOCK_STALE_MS } = options
let removedCount = 0

try {
// Check if directory exists
const dirStat = await fs.stat(directoryPath).catch(() => null)
if (!dirStat || !dirStat.isDirectory()) {
return 0
}

const entries = await fs.readdir(directoryPath, { withFileTypes: true })
const now = Date.now()

for (const entry of entries) {
const entryPath = path.join(directoryPath, entry.name)

// Check for .lock files/directories created by proper-lockfile
if (entry.name.endsWith(".lock")) {
try {
const stat = await fs.stat(entryPath)
const age = now - stat.mtimeMs

// Remove if older than stale duration
// Use a slightly larger threshold to account for filesystem timing quirks
if (age > staleDurationMs) {
if (stat.isDirectory()) {
await fs.rm(entryPath, { recursive: true, force: true })
} else {
await fs.unlink(entryPath)
}
removedCount++
console.log(
`[cleanupStaleLocks] Removed stale lock: ${entryPath} (age: ${Math.round(age / 1000)}s)`,
)
}
} catch (err) {
// Ignore errors for individual lock files - they may have been
// cleaned up by another process or the original operation completed
console.warn(`[cleanupStaleLocks] Could not process ${entryPath}:`, err)
}
} else if (recursive && entry.isDirectory()) {
// Recursively clean subdirectories (but not .lock directories themselves)
removedCount += await cleanupStaleLocks(entryPath, options)
}
}
} catch (err) {
// Log but don't throw - cleanup failures shouldn't block extension activation
console.error(`[cleanupStaleLocks] Error scanning ${directoryPath}:`, err)
}

return removedCount
}

export { safeWriteJson, cleanupStaleLocks, LOCK_STALE_MS }
Loading