Wait for file change when process exits#47427
Merged
tmat merged 1 commit intodotnet:release/9.0.3xxfrom Mar 11, 2025
Merged
Conversation
Member
Author
|
@phil-allen-msft ptal |
Contributor
There was a problem hiding this comment.
PR Overview
This PR fixes an issue where the file change is not awaited when a process exits. It includes changes to update the file change callback logic in HotReloadDotNetWatcher.cs and updates the ProcessCleanupTimeout configuration in EnvironmentVariables.cs.
- Updated file change callback behavior to wait for a file update on process exit.
- Modified ProcessCleanupTimeout configuration to use a default value of 5 seconds.
- Adjusted the ReadTimeSpan method to accept a default value for improved configurability.
Reviewed Changes
| File | Description |
|---|---|
| src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs | Updated the catch block to always wait for file change when the process exits. |
| src/BuiltInTools/dotnet-watch/EnvironmentVariables.cs | Modified ProcessCleanupTimeout to include a configurable default value and updated ReadTimeSpan accordingly. |
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs:223
- The removed assignment to false followed by setting the value to true in the catch block may cause confusion. Consider removing the commented-out assignment to clarify that the intended behavior is to wait for a file change when the process exits.
waitForFileChangeBeforeRestarting = false;
src/BuiltInTools/dotnet-watch/EnvironmentVariables.cs:51
- The original ReadTimeSpan method signature has been removed in favor of one that requires a default value. Please verify that all calls to ReadTimeSpan have been updated accordingly to prevent any unintended behavior.
private static TimeSpan ReadTimeSpan(string variableName)
phil-allen-msft
approved these changes
Mar 10, 2025
Open
3 tasks
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #47425.
There are two code paths that may be executed when process exists ,depending on exact timing of task cancellation.
One of the code paths was incorrectly not waiting for a file update before restarting the app.