Presets: prevent re-entrant reapply loop triggered by symlinked preset files #4670

hanniavalera · 2026-01-29T17:54:57Z

This change addresses item #4668

This pull request addresses a bug related to infinite reloading loops when using symlinked preset files in the presets system. The main change is the introduction of a guard to prevent the reapplyPresets method from being called recursively or concurrently, which could previously lead to an infinite loop.

This changes visible behavior and fixes bug

The following changes are proposed:

Added a guard (_isReapplyingPresets) in the PresetsController class to prevent multiple simultaneous executions of the reapplyPresets method, fixing infinite reload loops when preset files are symlinks or include symlinked files. [1] [2] [3]
Updated the changelog to document the fix for the infinite presets reloading loop with symlinked preset files.

Copilot

Pull request overview

Prevents an infinite presets reload loop (issue #4668) by adding re-entry protection to PresetsController.reapplyPresets(), and documents the fix.

Changes:

Added an _isReapplyingPresets guard to prevent concurrent/recursive reapplyPresets() execution.
Added unit tests intended to validate re-entry protection and symlink behaviors.
Updated changelog entry for the symlink-related infinite reload fix.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
src/presets/presetsController.ts	Adds a re-entry guard around `reapplyPresets()` to prevent recursive/concurrent reapply loops.
test/unit-tests/presets/presetsController.test.ts	Introduces tests around symlink handling and re-entry protection patterns.
CHANGELOG.md	Documents the bug fix for symlink-triggered infinite presets reload loops.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-29T19:54:25Z

src/presets/presetsController.ts

+        if (this._isReapplyingPresets) {
+            return;
+        }
+        this._isReapplyingPresets = true;

-        // reset all expanded presets storage.
-        this._referencedFiles = Array.from(referencedFiles.keys());
+        try {
+            const referencedFiles: Map<string, preset.PresetsFile | undefined> =
+                new Map();
+
+            // Reset all changes due to expansion since parents could change
+            await this._presetsParser.resetPresetsFiles(
+                referencedFiles,
+                this.project.workspaceContext.config.allowCommentsInPresetsFile,
+                this.project.workspaceContext.config.allowUnsupportedPresetsVersions
+            );

-        this.project.minCMakeVersion = preset.minCMakeVersion(this.folderPath);
+            // reset all expanded presets storage.
+            this._referencedFiles = Array.from(referencedFiles.keys());

-        if (this.project.configurePreset) {
-            await this.setConfigurePreset(this.project.configurePreset.name);
-        }
-        // Don't need to set build/test presets here since they are reapplied in setConfigurePreset
+            this.project.minCMakeVersion = preset.minCMakeVersion(this.folderPath);
+
+            if (this.project.configurePreset) {
+                await this.setConfigurePreset(this.project.configurePreset.name);
+            }
+            // Don't need to set build/test presets here since they are reapplied in setConfigurePreset

-        await this.watchPresetsChange();
+            await this.watchPresetsChange();
+        } finally {
+            this._isReapplyingPresets = false;
+        }


reapplyPresets() now returns early when a reapply is already in progress. This can drop legitimate file-watcher/config change events that happen during a long-running reapply, leaving presets state stale (the FileWatcher also debounces events for 500ms, increasing the chance of missing the follow-up event). Consider tracking a pending reapply (e.g., set a _reapplyPresetsPending flag when re-entry is detected) and running one more reapply in finally after clearing _isReapplyingPresets.

Copilot · 2026-01-29T19:54:25Z

test/unit-tests/presets/presetsController.test.ts

+/* eslint-disable no-unused-expressions */
+import { expect } from 'chai';
+import * as path from 'path';
+import * as fs from 'fs';
+import * as os from 'os';


This test imports expect directly from chai, but most preset-related tests use the shared @test/util export (which also configures chai plugins like chai-as-promised). To keep assertions/config consistent across the test suite, import expect from @test/util here as well.

Copilot · 2026-01-29T19:54:27Z

test/unit-tests/presets/presetsController.test.ts

+            // Remove symlink first if it exists
+            if (symlinkPath && fs.existsSync(symlinkPath)) {
+                try {
+                    // On Windows, junctions need special handling
+                    const stats = fs.lstatSync(symlinkPath);
+                    if (stats.isSymbolicLink() || stats.isDirectory()) {
+                        if (process.platform === 'win32') {
+                            // Use rmdir for junctions on Windows
+                            fs.rmdirSync(symlinkPath);
+                        } else {
+                            fs.unlinkSync(symlinkPath);
+                        }
+                    }
+                } catch {
+                    // Ignore cleanup errors
+                }
+            }
+            // Remove the presets directory and its contents
+            if (fs.existsSync(presetsDir)) {
+                if (fs.existsSync(presetsJsonPath)) {
+                    fs.unlinkSync(presetsJsonPath);
+                }
+                fs.rmdirSync(presetsDir);
+            }
+            // Remove the temp directory
+            try {
+                fs.rmdirSync(tempDir);
+            } catch {
+                // Ignore if not empty or already deleted


Test cleanup uses fs.rmdirSync/manual unlinking, which is error-prone and can make tests flaky if a test fails before removing all created files (e.g., the include file). Other tests in this repo use fs.rmSync(..., { recursive: true, force: true }); consider switching to that pattern to reliably clean up tempDir and avoid teardown failures.

Suggested change

// Remove symlink first if it exists

if (symlinkPath && fs.existsSync(symlinkPath)) {

try {

// On Windows, junctions need special handling

const stats = fs.lstatSync(symlinkPath);

if (stats.isSymbolicLink() || stats.isDirectory()) {

if (process.platform === 'win32') {

// Use rmdir for junctions on Windows

fs.rmdirSync(symlinkPath);

} else {

fs.unlinkSync(symlinkPath);

}

}

} catch {

// Ignore cleanup errors

}

}

// Remove the presets directory and its contents

if (fs.existsSync(presetsDir)) {

if (fs.existsSync(presetsJsonPath)) {

fs.unlinkSync(presetsJsonPath);

}

fs.rmdirSync(presetsDir);

}

// Remove the temp directory

try {

fs.rmdirSync(tempDir);

} catch {

// Ignore if not empty or already deleted

try {

fs.rmSync(tempDir, { recursive: true, force: true });

} catch {

// Ignore cleanup errors

Copilot · 2026-01-29T19:54:27Z

test/unit-tests/presets/presetsController.test.ts

+    /**
+     * Test the re-entry prevention logic pattern used in PresetsController.
+     * This is a simplified simulation of the fix for issue #4668.
+     */
+    test('Re-entry prevention flag blocks concurrent calls', async () => {
+        // Simulate the _isReapplyingPresets flag pattern
+        let isReapplyingPresets = false;
+        let reapplyCallCount = 0;
+        const callOrder: string[] = [];
+
+        // Simulate the reapplyPresets function with the fix
+        const reapplyPresets = async (): Promise<void> => {
+            if (isReapplyingPresets) {
+                callOrder.push('blocked');
+                return;
+            }
+            isReapplyingPresets = true;
+            reapplyCallCount++;
+            callOrder.push('started');
+
+            try {
+                // Simulate async work that might trigger file watcher events
+                await new Promise(resolve => setTimeout(resolve, 10));
+
+                // Simulate a recursive call (as would happen from file watcher)
+                await reapplyPresets();
+
+                callOrder.push('completed');
+            } finally {
+                isReapplyingPresets = false;
+            }
+        };
+
+        // Call reapplyPresets
+        await reapplyPresets();
+
+        // Verify only one actual execution happened
+        expect(reapplyCallCount).to.equal(1);
+
+        // Verify the call order shows the recursive call was blocked
+        expect(callOrder).to.deep.equal(['started', 'blocked', 'completed']);
+    });


The tests titled around “re-entry prevention” only simulate a local boolean and never exercise PresetsController.reapplyPresets() itself. That means the new _isReapplyingPresets behavior (and the symlink-triggered loop regression) isn’t actually covered. Consider constructing a PresetsController with a stubbed/mocked PresetsParser and calling reapplyPresets() twice (including a re-entrant call path) to assert the guard works in the real implementation.

Copilot · 2026-01-29T19:54:27Z

test/unit-tests/presets/presetsController.test.ts

+    test('Symlinked preset files can be read correctly', function () {
+        if (!symlinkSupported) {
+            this.skip();
+            return;
+        }
+
+        // Create an include file in the presets directory
+        const includeFilePath = path.join(presetsDir, 'include-presets.json');
+        const includeContent = JSON.stringify({
+            version: 3,
+            configurePresets: [
+                {
+                    name: "included-preset",
+                    displayName: "Included Preset",
+                    generator: "Ninja"
+                }
+            ]
+        }, null, 2);
+        fs.writeFileSync(includeFilePath, includeContent);
+
+        // Update the main presets file to include the other file
+        const mainPresets = {
+            version: 3,
+            include: ['include-presets.json'],
+            configurePresets: [
+                {
+                    name: "main-preset",
+                    displayName: "Main Preset",
+                    generator: "Ninja"
+                }
+            ]
+        };
+        fs.writeFileSync(presetsJsonPath, JSON.stringify(mainPresets, null, 2));
+
+        // Get the path through the symlink/junction
+        const includeFileViaSym = process.platform === 'win32'
+            ? path.join(symlinkPath, 'include-presets.json')
+            : presetsJsonPath; // On Unix, symlinkPath points directly to the file
+


In Symlinked preset files can be read correctly, the Unix path (includeFileViaSym) is set to presetsJsonPath (the main file), so the test doesn't actually read the include file through any symlink on Unix and effectively only asserts anything on Windows. Either adjust the setup so the include file is accessed via a symlinked directory on Unix too, or rename/scope the test so it only claims to validate what it actually checks.

attempt at fixing infinite preset loop, added new tests to validate

b5034bf

hanniavalera linked an issue Jan 29, 2026 that may be closed by this pull request

[Bug] Infinity presets reloading #4668

Open

fixed tests for macos

5624be1

hanniavalera requested review from Copilot and gcampbell-msft January 29, 2026 19:51

Copilot started reviewing on behalf of hanniavalera January 29, 2026 19:51 View session

Copilot AI reviewed Jan 29, 2026

View reviewed changes

changing approach to adding a startup grace period for filewatcher class

6a658bc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Presets: prevent re-entrant reapply loop triggered by symlinked preset files #4670

Presets: prevent re-entrant reapply loop triggered by symlinked preset files #4670

Uh oh!

hanniavalera commented Jan 29, 2026

Copilot AI left a comment

Copilot AI Jan 29, 2026

Copilot AI Jan 29, 2026

Copilot AI Jan 29, 2026

Copilot AI Jan 29, 2026

Copilot AI Jan 29, 2026

Labels

2 participants

Presets: prevent re-entrant reapply loop triggered by symlinked preset files #4670

Are you sure you want to change the base?

Presets: prevent re-entrant reapply loop triggered by symlinked preset files #4670

Uh oh!

Conversation

hanniavalera commented Jan 29, 2026

This change addresses item #4668

This changes visible behavior and fixes bug

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

Labels

2 participants