fix: prevent race conditions in SettingsManager setCwd and debounce (#485)

## Summary

- **Concurrent `setCwd()` calls** could produce duplicate file watchers
because both calls cleared `initialized` before either completed
`initialize()`. Added an `initPromise` guard so concurrent calls await
the same initialization.
- **Debounce timer callback** could fire after `dispose()`, reloading
stale settings and calling `onChange` on a disposed manager. Added a
`disposed` flag checked in the async callback before proceeding.

Follows up on #454 which fixed the missing `dispose()` calls — this PR
hardens the `SettingsManager` lifecycle against race conditions.

## Test plan

- [x] `npm run lint` passes
- [x] All settings tests pass
- [x] Verified `disposed` flag prevents stale callback execution
- [x] Verified `initPromise` deduplicates concurrent `initialize()`
calls
This commit is contained in:
tbounsiar
2026-03-27 13:12:19 +01:00
committed by GitHub
parent ae6c388314
commit 7506223cff

View File

@@ -92,7 +92,9 @@ export class SettingsManager {
private onChange?: () => void;
private logger: { log: (...args: any[]) => void; error: (...args: any[]) => void };
private initialized = false;
private disposed = false;
private debounceTimer: ReturnType<typeof setTimeout> | null = null;
private initPromise: Promise<void> | null = null;
constructor(cwd: string, options?: SettingsManagerOptions) {
this.cwd = cwd;
@@ -107,10 +109,19 @@ export class SettingsManager {
if (this.initialized) {
return;
}
if (this.initPromise) {
return this.initPromise;
}
await this.loadAllSettings();
this.setupWatchers();
this.initialized = true;
this.disposed = false;
this.initPromise = this.loadAllSettings().then(() => {
if (!this.disposed) {
this.setupWatchers();
this.initialized = true;
}
this.initPromise = null;
});
return this.initPromise;
}
/**
@@ -235,9 +246,14 @@ export class SettingsManager {
this.debounceTimer = setTimeout(async () => {
this.debounceTimer = null;
if (this.disposed) {
return;
}
try {
await this.loadAllSettings();
this.onChange?.();
if (!this.disposed) {
this.onChange?.();
}
} catch (error) {
this.logger.error("Failed to reload settings:", error);
}
@@ -268,7 +284,6 @@ export class SettingsManager {
this.dispose();
this.cwd = cwd;
this.initialized = false;
await this.initialize();
}
@@ -276,6 +291,10 @@ export class SettingsManager {
* Disposes of file watchers and cleans up resources
*/
dispose(): void {
this.disposed = true;
this.initialized = false;
this.initPromise = null;
if (this.debounceTimer) {
clearTimeout(this.debounceTimer);
this.debounceTimer = null;
@@ -285,6 +304,5 @@ export class SettingsManager {
watcher.close();
}
this.watchers = [];
this.initialized = false;
}
}