Skip to content

Use non-recursive watcher in PhysicalFilesWatcher for missing parent directories#126026

Draft
svick wants to merge 28 commits intomainfrom
copilot/handle-non-existent-directory-watcher
Draft

Use non-recursive watcher in PhysicalFilesWatcher for missing parent directories#126026
svick wants to merge 28 commits intomainfrom
copilot/handle-non-existent-directory-watcher

Conversation

@svick
Copy link
Member

@svick svick commented Mar 24, 2026

Fixes #116713

Problem

When PhysicalFileProvider is constructed with a root directory that does not yet exist (e.g., a configuration file path whose parent directory hasn't been created), Watch() fails because FileSystemWatcher cannot watch a non-existent directory. This commonly occurs with AddJsonFile when the config file's parent directory is missing at startup.

Solution

PhysicalFilesWatcher now gracefully handles a missing root directory by deferring FileSystemWatcher activation until the root appears. A PendingCreationWatcher monitors the nearest existing ancestor directory using a non-recursive FileSystemWatcher and cascades through intermediate directory levels as they are created. Once the root directory exists, the main recursive FileSystemWatcher is enabled and any already-existing watched entries are reported.

Callers always receive normal FSW-backed change tokens — no re-registration is needed when the root directory appears later.

Changes

File Description
PhysicalFilesWatcher.cs Added PendingCreationWatcher inner class that watches for a non-existent directory to be created. TryEnableFileSystemWatcher defers to EnsureRootCreationWatcher when _root doesn't exist, with a callback to retry once it appears. ReportExistingWatchedEntries fires tokens for entries created before the FSW was active. The constructor normalizes _root to always have a trailing separator. OnFileSystemEntryChange now uses DirectoryInfo for directory paths so exclusion filters work correctly.
PhysicalFileProvider.cs Constructor no longer throws DirectoryNotFoundException for a missing root — the PhysicalFilesWatcher handles it. Updated Watch doc comments to include directories. Removed duplicate _pathSeparators field in favor of PathUtils.PathSeparators.
FileConfigurationSource.cs ResolveFileProvider creates the PhysicalFileProvider with the file's immediate parent directory (even if missing), relying on the watcher to handle the non-existent case.
PollingFileChangeToken.cs GetLastWriteTimeUtc now falls back to checking DirectoryInfo when FileInfo.Exists is false, so polling correctly detects directory changes.
PathUtils.cs PathSeparators made internal for reuse across files.
PhysicalFilesWatcherTests.cs Tests for missing root (file path and wildcard), root deleted and recreated, subdirectory create/delete/recreate cycles, directory watch tokens, hidden directory exclusion, and active polling variants.
FileConfigurationProviderTest.cs Integration test verifying the watch token fires when a file is created inside a previously-missing directory.
PhysicalFileProviderTests.cs Fixed TokenFiredForGlobbingPatternsPointingToSubDirectory path length issue on .NET Framework. Fixed TokensWithForwardAndBackwardSlashesAreSame and TokensFiredForNewDirectoryContentsOnRename.

…t directories

When a watched file's parent directory does not exist, PhysicalFilesWatcher
now uses a cascading non-recursive FileSystemWatcher (PendingCreationWatcher)
that advances through intermediate directory levels as they are created. This
avoids adding recursive watches on the entire ancestor tree and avoids
spurious token fires for intermediate directory creations.

Key changes:
- PendingCreationWatcher cascades through missing directory levels using a
  non-recursive FileSystemWatcher, only firing when the target file is created
- HasMissingParentDirectory detects when the watched path's parent does not exist
- FileConfigurationSource.ResolveFileProvider walks up to the nearest existing ancestor
- ArraySegmentExtensions added for netstandard2.0/net462 compatibility

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses excessive recursive OS file watches (e.g., inotify) triggered when watching a file whose parent directory doesn’t exist yet, by introducing a non-recursive “cascading” watcher approach and adjusting configuration/file-provider resolution to work with missing directories.

Changes:

  • Add a PendingCreationWatcher path-component–by–path-component watcher for missing-parent scenarios, avoiding recursive subtree watches.
  • Allow PhysicalFileProvider construction when the root directory doesn’t exist and adjust FileSystemWatcher initialization accordingly.
  • Update FileConfigurationSource.ResolveFileProvider behavior and add tests covering missing-directory watch behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
src/libraries/Microsoft.Extensions.FileProviders.Physical/src/PhysicalFilesWatcher.cs Adds pending-creation watcher logic and changes when the main FileSystemWatcher is enabled.
src/libraries/Microsoft.Extensions.FileProviders.Physical/src/PhysicalFileProvider.cs Removes root existence requirement; creates FileSystemWatcher without setting Path up-front.
src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationSource.cs Changes resolution to create a provider for the file’s immediate directory and reduce the config path to the filename.
src/libraries/Microsoft.Extensions.FileProviders.Physical/src/ArraySegmentExtensions.cs Adds ArraySegment<T>.At and a Slice polyfill for non-NET TFMs.
src/libraries/Microsoft.Extensions.FileProviders.Physical/tests/PhysicalFilesWatcherTests.cs Adds unit tests for pending-creation watcher behavior and main watcher enablement.
src/libraries/Microsoft.Extensions.Configuration.FileExtensions/tests/FileConfigurationProviderTest.cs Adds an integration-style test for config reload token firing when missing directory/file is created.

Copilot AI review requested due to automatic review settings March 24, 2026 14:23
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Petr Onderka <gsvick@gmail.com>
Copilot AI review requested due to automatic review settings March 24, 2026 17:08
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

Copilot AI review requested due to automatic review settings March 25, 2026 17:11
@svick svick force-pushed the copilot/handle-non-existent-directory-watcher branch from f590fea to 7963837 Compare March 25, 2026 17:11
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Copilot AI review requested due to automatic review settings March 26, 2026 12:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copilot AI review requested due to automatic review settings March 26, 2026 14:47
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

@github-actions

This comment has been minimized.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 26, 2026 15:03
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copilot AI review requested due to automatic review settings March 26, 2026 17:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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


private static readonly char[] _pathSeparators = new[]
{Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar};
internal static char[] PathSeparators =>
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

PathSeparators currently creates a new char[] on every access ([ ... ]). This introduces avoidable allocations on hot paths like GetFileInfo, Watch, and PathNavigatesAboveRoot. Consider making this a cached immutable instance (e.g., internal static readonly char[] PathSeparators = { ... }; or a get-only property backed by a static field) so callers can reuse it without per-call allocations.

Suggested change
internal static char[] PathSeparators =>
internal static readonly char[] PathSeparators =

Copilot uses AI. Check for mistakes.
Comment on lines +495 to +509
try
{
if (string.IsNullOrEmpty(_fileWatcher.Path))
{
_fileWatcher.Path = _root;
}

_fileWatcher.EnableRaisingEvents = true;

// Only scan for existing entries if the FSW was enabled after _root
// was initially missing (i.e. we went through the PCW path). In the
// normal case where _root always existed, there is no gap to cover.
justEnabledAfterRootCreated = _rootWasUnavailable;
_rootWasUnavailable = false;
}
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

After the root directory is successfully created and the main FileSystemWatcher is enabled, _rootCreationWatcher is left referenced (with a cancelled token) until PhysicalFilesWatcher.Dispose() or another missing-root cycle. Consider disposing and clearing _rootCreationWatcher once _fileWatcher.EnableRaisingEvents = true succeeds to avoid keeping the CancellationTokenSource/registration alive for the lifetime of the provider.

Copilot uses AI. Check for mistakes.
Comment on lines +33 to +41
private static PhysicalFilesWatcher CreateWatcher(string rootPath, bool useActivePolling)
{
FileSystemWatcher? fsw = useActivePolling ? null : new FileSystemWatcher(rootPath);
var watcher = new PhysicalFilesWatcher(rootPath, fsw, pollForChanges: useActivePolling);
if (useActivePolling)
{
watcher.UseActivePolling = true;
}
return watcher;
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

CreateWatcher constructs new FileSystemWatcher(rootPath) when useActivePolling == false, which requires the root to exist and doesn’t exercise the new production path where PhysicalFileProvider creates new FileSystemWatcher() with an empty Path (to support missing roots). To better cover the missing-root scenario and the _fileWatcher.Path initialization in TryEnableFileSystemWatcher, consider constructing new FileSystemWatcher() here as well (and letting PhysicalFilesWatcher set the path when enabling).

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Contributor

🤖 Copilot Code Review — PR #126026

Holistic Assessment

Motivation: The problem is real and well-motivated. PhysicalFileProvider previously required the root directory to exist at construction time and FileConfigurationSource.ResolveFileProvider() walked up to the nearest existing ancestor, which caused issues when config files live in directories created after app startup (e.g., container volumes, dynamic deployments). This addresses a genuine gap.

Approach: The approach — deferring FileSystemWatcher enablement and using a cascading PendingCreationWatcher to detect each directory level being created — is fundamentally sound. The PendingCreationWatcher design is careful about race conditions (post-start checks, sender validation, lock release before token registration). However, the PR is large (~780 lines across 9 files, 29 commits) and mixes a breaking behavioral change (DirectoryNotFoundException removal) with the new functionality, which warrants careful human scrutiny.

Summary: ⚠️ Needs Human Review. The core watching mechanism appears correct and well-tested. However, several concerns require human judgment: (1) the PathUtils.PathSeparators property allocates a new array on every call in hot paths — a performance regression from the previous static readonly field, (2) removing DirectoryNotFoundException from the PhysicalFileProvider constructor is a breaking change that should be explicitly acknowledged and documented, (3) ReportExistingWatchedEntries recursively enumerates the entire directory tree which could be slow for large trees, and (4) the FileConfigurationSource.ResolveFileProvider() behavioral change (no longer walking up to find the nearest existing ancestor) is significant. Human reviewers should also evaluate whether the PendingCreationWatcher complexity (~230 lines) is the right tradeoff versus simpler alternatives.


Detailed Findings

⚠️ Performance — PathUtils.PathSeparators allocates a new array on every call

PathUtils.PathSeparators (PathUtils.cs line 29) is now defined as:

internal static char[] PathSeparators =>
    [Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar];

This creates a new char[2] array on every property access. The previous code used a cached private static readonly char[] field. This property is called in hot paths:

  • PhysicalFileProvider.GetFileInfo() (line 265) — called on every file lookup
  • PhysicalFileProvider.GetDirectoryContents() (line 310) — called on every directory enumeration
  • PhysicalFileProvider.Watch() (line 357) — called on every watch registration
  • PathUtils.PathNavigatesAboveRoot() (line 45) — called on every path validation
  • PendingCreationWatcher.GetChildName() (lines 738–739) — two calls per invocation
  • PendingCreationWatcher.IsTargetDirectory() (lines 746–747) — two calls per invocation

The property should be changed back to a static readonly field to avoid per-call allocations. If internal visibility is needed, make the field internal static readonly:

internal static readonly char[] PathSeparators =
    [Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar];

Note: callers that pass it to TrimStart/TrimEnd/IndexOfAny on ReadOnlySpan<char> should be fine with a shared array since those methods don't modify the array.

⚠️ Breaking Change — DirectoryNotFoundException removed from PhysicalFileProvider constructor

The PhysicalFileProvider constructor (PhysicalFileProvider.cs, lines 59–64) previously threw DirectoryNotFoundException when root didn't exist:

// REMOVED:
if (!Directory.Exists(Root))
{
    throw new DirectoryNotFoundException(Root);
}

This is a public API behavioral change. Code that catches DirectoryNotFoundException to validate directory existence will silently succeed now. This should be:

  1. Explicitly documented as a breaking change (per dotnet/runtime process)
  2. Called out in the PR description as intentional
  3. Filed in the breaking change tracker if not already

A human reviewer should decide whether this breaking change is acceptable for the target release.

⚠️ Behavioral Change — FileConfigurationSource.ResolveFileProvider() no longer walks up directory tree

The old ResolveFileProvider() walked up the directory tree to find the nearest existing ancestor:

// OLD: walked up to find existing ancestor, composed relative path
while (!string.IsNullOrEmpty(directory) && !Directory.Exists(directory))
{
    pathToFile = Path.Combine(Path.GetFileName(directory), pathToFile);
    directory = Path.GetDirectoryName(directory);
}

The new code uses the direct parent:

// NEW: uses direct parent, relies on PhysicalFileProvider handling non-existent dirs
FileProvider = new PhysicalFileProvider(directory);
Path = System.IO.Path.GetFileName(Path);

This is a significant behavioral change. For example, with config path /app/config/missing/appsettings.json where /app/config exists but missing/ does not:

  • Old: PhysicalFileProvider root = /app/config/, Path = missing/appsettings.json
  • New: PhysicalFileProvider root = /app/config/missing/, Path = appsettings.json

The new behavior relies entirely on the PendingCreationWatcher mechanism to watch for the non-existent directory. This is likely correct but is a meaningful semantic change in how the file provider is rooted. A human reviewer should verify this doesn't break any downstream consumer expectations.

⚠️ Performance — ReportExistingWatchedEntries enumerates entire directory tree

ReportExistingWatchedEntries() (PhysicalFilesWatcher.cs, lines 425–447) calls:

Directory.EnumerateFileSystemEntries(_root, "*", SearchOption.AllDirectories)

This recursively enumerates every file and directory under _root. For large directory trees (e.g., a node_modules-heavy project or a large deployment), this could block the calling thread for a significant time. The method is called synchronously from TryEnableFileSystemWatcher after the root directory appears.

The early-exit optimization (break when both dictionaries are empty, line 436–439) helps, but only after all watched entries have been matched. Consider:

  • Limiting the enumeration depth or count
  • Or documenting this as expected behavior since it only happens once when a previously-missing root appears

✅ Thread Safety — Locking protocol is sound

Verified the locking protocol:

  • _rootWasUnavailable is only accessed under _fileWatcherLock (lines 491, 507, 508, 517) — safe
  • _rootCreationWatcherLock is released before Token.Register() (line 581) to prevent deadlocks with synchronous callback invocation — correct
  • _fileWatcherLock is released before EnsureRootCreationWatcher() (line 536) so re-entrant calls from token callbacks can acquire it — correct
  • PendingCreationWatcher._lock protects _watcher and _expectedName — correct
  • PendingCreationWatcher.Dispose() uses Interlocked.Exchange for atomic disposed check — correct
  • No circular lock dependencies exist between the three lock objects

✅ Race Condition Handling — PendingCreationWatcher is well-designed

The SetupWatcherNoLock method (lines 755–817) correctly handles the race between directory creation and watcher setup:

  1. Fast-forwards through already-existing directories (lines 760–772)
  2. Creates the FSW and enables events (lines 775–793)
  3. Post-start race check (lines 798–802): re-checks if the directory appeared during setup
  4. If it appeared, disposes the watcher and loops to the next level

The OnCreated callback (lines 824–857) validates sender != _watcher to ignore stale events from disposed watchers. This is a correct and necessary check.

✅ Resource Management — PendingCreationWatcher disposal is correct

  • Dispose() acquires _lock, nulls _watcher, then disposes it outside the lock — prevents callbacks from using a disposed watcher
  • _cts.Dispose() is called after the watcher is disposed — correct order
  • PhysicalFilesWatcher.Dispose() disposes _rootCreationWatcher under _rootCreationWatcherLock — correct
  • ObjectDisposedException catch in EnsureRootCreationWatcher (line 583) handles the race between Dispose() and Token.Register() — correct

PollingFileChangeToken directory support — Correct

The GetLastWriteTimeUtc() method (PollingFileChangeToken.cs, lines 47–65) now checks for both file and directory existence. The lazy _directoryInfo creation (line 56) avoids allocating DirectoryInfo when the path is a file. The Refresh() calls ensure fresh state.

DirectoryInfoWrapper.Refresh() — Correct and necessary

The added _directoryInfo.Refresh() call (DirectoryInfoWrapper.cs, line 38) ensures the Exists property reflects the current state, not stale cached state. This is needed because the DirectoryInfo may have been created when the directory didn't exist.

✅ Test Coverage — Comprehensive

The PR adds 10 new test methods covering:

  • Missing parent directory (single and multiple levels)
  • Directory deleted and recreated
  • Root directory deleted and recreated
  • Wildcard patterns with missing root and prefix directories
  • Directory creation detection
  • Hidden directory exclusion

Tests use [Theory] with [MemberData] to cover both FSW and polling modes — good practice.

💡 Suggestion — OnRenamed exception pattern inconsistency (pre-existing)

OnRenamed (line 313–317) still uses the old || pattern for exception catching:

ex is IOException ||
ex is SecurityException ||

while OnFileSystemEntryChange (line 370) and ReportExistingWatchedEntries (line 442) use the modern or pattern. This is pre-existing and out of scope for this PR, but could be cleaned up.

💡 Suggestion — PhysicalFilesWatcher constructor trailing separator normalization

The constructor (lines 96–98) normalizes the trailing separator:

_root = root.Length > 0 && root[root.Length - 1] != Path.DirectorySeparatorChar && root[root.Length - 1] != Path.AltDirectorySeparatorChar
    ? root + Path.DirectorySeparatorChar
    : root;

This duplicates logic from PathUtils.EnsureTrailingSlash. Consider reusing PathUtils.EnsureTrailingSlash(root) for consistency and maintainability.


Review generated by Copilot (Claude Opus 4.6). Multi-model cross-check with GPT-5.4 was attempted but did not complete within the timeout window.

Generated by Code Review for issue #126026 ·

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FileConfigurationProvider watches all files starting an first existing path; /foo/appsettings.json will listen on all files on the file system

2 participants