From 380535e9b98c55c6b33e4ebc5f7b100fe1530a10 Mon Sep 17 00:00:00 2001 From: David Date: Fri, 27 Sep 2024 15:24:27 -0400 Subject: [PATCH] fix: Fix UpdateFileAsync requesting HR before VS detects file change --- .../HotReload/ServerHotReloadProcessor.cs | 75 ++++++++++++++++--- .../ClientHotReloadProcessor.ClientApi.cs | 2 +- .../ClientHotReloadProcessor.Common.Status.cs | 2 +- ...ClientHotReloadProcessor.MetadataUpdate.cs | 4 + 4 files changed, 71 insertions(+), 12 deletions(-) diff --git a/src/Uno.UI.RemoteControl.Server.Processors/HotReload/ServerHotReloadProcessor.cs b/src/Uno.UI.RemoteControl.Server.Processors/HotReload/ServerHotReloadProcessor.cs index 292915003111..1a44160934f2 100644 --- a/src/Uno.UI.RemoteControl.Server.Processors/HotReload/ServerHotReloadProcessor.cs +++ b/src/Uno.UI.RemoteControl.Server.Processors/HotReload/ServerHotReloadProcessor.cs @@ -28,7 +28,7 @@ partial class ServerHotReloadProcessor : IServerProcessor, IDisposable { private FileSystemWatcher[]? _watchers; private CompositeDisposable? _watcherEventsDisposable; - private IRemoteControlServer _remoteControlServer; + private readonly IRemoteControlServer _remoteControlServer; public ServerHotReloadProcessor(IRemoteControlServer remoteControlServer) { @@ -219,6 +219,7 @@ private class HotReloadServerOperation private ImmutableHashSet _filePaths; private int /* HotReloadResult */ _result = -1; private CancellationTokenSource? _deferredCompletion; + private int _noChangesRetry; public long Id { get; } = Interlocked.Increment(ref _count); @@ -290,6 +291,12 @@ public bool TryMerge(ImmutableHashSet filePaths) } } + /// + /// Configure a simple auto-retry strategy if no changes are detected. + /// + public void EnableAutoRetryIfNoChanges() + => _noChangesRetry = 1; + /// /// As errors might get a bit after the complete from the IDE, we can defer the completion of the operation. /// @@ -313,6 +320,14 @@ public ValueTask Complete(HotReloadServerResult result, Exception? exception = n private async ValueTask Complete(HotReloadServerResult result, Exception? exception, bool isFromNext) { + if (_result is -1 + && result is HotReloadServerResult.NoChanges + && Interlocked.Decrement(ref _noChangesRetry) >= 0 + && await _owner.RequestHotReloadToIde(Id)) + { + return; + } + Debug.Assert(result != HotReloadServerResult.InternalError || exception is not null); // For internal error we should always provide an exception! // Remove this from current @@ -441,13 +456,14 @@ private async Task ProcessUpdateFile(UpdateFile message) var (result, error) = message switch { { FilePath: null or { Length: 0 } } => (FileUpdateResult.BadRequest, "Invalid request (file path is empty)"), - { OldText: not null, NewText: not null } => DoUpdate(message.OldText, message.NewText), - { OldText: null, NewText: not null } => DoWrite(message.NewText), - { NewText: null, IsCreateDeleteAllowed: true } => DoDelete(), + { OldText: not null, NewText: not null } => await DoUpdate(message.OldText, message.NewText), + { OldText: null, NewText: not null } => await DoWrite(message.NewText), + { NewText: null, IsCreateDeleteAllowed: true } => await DoDelete(), _ => (FileUpdateResult.BadRequest, "Invalid request") }; if ((int)result < 300 && !message.IsForceHotReloadDisabled) { + hotReload.EnableAutoRetryIfNoChanges(); await RequestHotReloadToIde(hotReload.Id); } @@ -459,7 +475,7 @@ private async Task ProcessUpdateFile(UpdateFile message) await _remoteControlServer.SendFrame(new UpdateFileResponse(message.RequestId, message.FilePath ?? "", FileUpdateResult.Failed, ex.Message)); } - (FileUpdateResult, string?) DoUpdate(string oldText, string newText) + async ValueTask<(FileUpdateResult, string?)> DoUpdate(string oldText, string newText) { if (!File.Exists(message.FilePath)) { @@ -471,7 +487,7 @@ private async Task ProcessUpdateFile(UpdateFile message) return (FileUpdateResult.FileNotFound, $"Requested file '{message.FilePath}' does not exists."); } - var originalContent = File.ReadAllText(message.FilePath); + var originalContent = await File.ReadAllTextAsync(message.FilePath); if (this.Log().IsEnabled(LogLevel.Trace)) { this.Log().LogTrace($"Original content: {originalContent} [{message.RequestId}]."); @@ -493,11 +509,14 @@ private async Task ProcessUpdateFile(UpdateFile message) return (FileUpdateResult.NoChanges, null); } - File.WriteAllText(message.FilePath, updatedContent); + var effectiveUpdate = WaitForFileUpdated(message.FilePath); + await File.WriteAllTextAsync(message.FilePath, updatedContent); + await effectiveUpdate; + return (FileUpdateResult.Success, null); } - (FileUpdateResult, string?) DoWrite(string newText) + async ValueTask<(FileUpdateResult, string?)> DoWrite(string newText) { if (!message.IsCreateDeleteAllowed && !File.Exists(message.FilePath)) { @@ -514,11 +533,14 @@ private async Task ProcessUpdateFile(UpdateFile message) this.Log().LogTrace($"Write content: {newText} [{message.RequestId}]."); } - File.WriteAllText(message.FilePath, newText); + var effectiveUpdate = WaitForFileUpdated(message.FilePath); + await File.WriteAllTextAsync(message.FilePath, newText); + await effectiveUpdate; + return (FileUpdateResult.Success, null); } - (FileUpdateResult, string?) DoDelete() + async ValueTask<(FileUpdateResult, string?)> DoDelete() { if (!File.Exists(message.FilePath)) { @@ -530,9 +552,41 @@ private async Task ProcessUpdateFile(UpdateFile message) return (FileUpdateResult.FileNotFound, $"Requested file '{message.FilePath}' does not exists."); } + var effectiveUpdate = WaitForFileUpdated(message.FilePath); File.Delete(message.FilePath); + await effectiveUpdate; + return (FileUpdateResult.Success, null); } + + static async Task WaitForFileUpdated(string path) + { + var file = new FileInfo(path); + var dir = file.Directory; + while (dir is { Exists: false }) + { + dir = dir.Parent; + } + + if (dir is null) + { + return; + } + + var tcs = new TaskCompletionSource(); + using var watcher = new FileSystemWatcher(dir.FullName); + watcher.Changed += async (snd, e) => + { + if (e.FullPath.Equals(file.FullName, StringComparison.OrdinalIgnoreCase)) + { + await Task.Delay(500); + tcs.TrySetResult(); + } + }; + watcher.EnableRaisingEvents = true; + + await Task.WhenAny(tcs.Task, Task.Delay(TimeSpan.FromSeconds(2))); + } } private async Task RequestHotReloadToIde(long sequenceId) @@ -545,6 +599,7 @@ private async Task RequestHotReloadToIde(long sequenceId) using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(2)); await using var ctReg = cts.Token.Register(() => hrRequested.TrySetCanceled()); + _pendingHotReloadRequestToIde.TryAdd(hrRequest.CorrelationId, hrRequested); await _remoteControlServer.SendMessageToIDEAsync(hrRequest); return await hrRequested.Task is { IsSuccess: true }; diff --git a/src/Uno.UI.RemoteControl/HotReload/ClientHotReloadProcessor.ClientApi.cs b/src/Uno.UI.RemoteControl/HotReload/ClientHotReloadProcessor.ClientApi.cs index 8c23bdaa6f62..fe911e17b0e5 100644 --- a/src/Uno.UI.RemoteControl/HotReload/ClientHotReloadProcessor.ClientApi.cs +++ b/src/Uno.UI.RemoteControl/HotReload/ClientHotReloadProcessor.ClientApi.cs @@ -61,7 +61,7 @@ public record struct UpdateRequest( /// The max delay to wait for the local application to process a hot-reload delta. /// /// This includes the time to apply the delta locally and then to run all local handlers. - public TimeSpan LocalHotReloadTimeout { get; set; } = TimeSpan.FromSeconds(3); + public TimeSpan LocalHotReloadTimeout { get; set; } = TimeSpan.FromSeconds(5); public UpdateRequest WithExtendedTimeouts(float? factor = null) { diff --git a/src/Uno.UI.RemoteControl/HotReload/ClientHotReloadProcessor.Common.Status.cs b/src/Uno.UI.RemoteControl/HotReload/ClientHotReloadProcessor.Common.Status.cs index e51c6ffc66cb..8fad9fe3bb56 100644 --- a/src/Uno.UI.RemoteControl/HotReload/ClientHotReloadProcessor.Common.Status.cs +++ b/src/Uno.UI.RemoteControl/HotReload/ClientHotReloadProcessor.Common.Status.cs @@ -287,7 +287,7 @@ internal void ReportCompleted() EndTime = DateTimeOffset.Now; _onUpdated(); } - else + else if (_result != (int)HotReloadClientResult.Ignored) // ReportIgnored auto completes but caller usually does not expect it (use ReportCompleted in finally) { Debug.Fail("The result should not have already been set."); } diff --git a/src/Uno.UI.RemoteControl/HotReload/ClientHotReloadProcessor.MetadataUpdate.cs b/src/Uno.UI.RemoteControl/HotReload/ClientHotReloadProcessor.MetadataUpdate.cs index 5c83f4fd5239..82992fec059b 100644 --- a/src/Uno.UI.RemoteControl/HotReload/ClientHotReloadProcessor.MetadataUpdate.cs +++ b/src/Uno.UI.RemoteControl/HotReload/ClientHotReloadProcessor.MetadataUpdate.cs @@ -19,6 +19,7 @@ using Microsoft.UI.Xaml.Input; using Microsoft.UI.Xaml.Media; using Uno.Diagnostics.UI; +using Uno.Threading; #if HAS_UNO_WINUI using _WindowActivatedEventArgs = Microsoft.UI.Xaml.WindowActivatedEventArgs; @@ -31,6 +32,7 @@ namespace Uno.UI.RemoteControl.HotReload; partial class ClientHotReloadProcessor { private static int _isWaitingForTypeMapping; + private static readonly AsyncLock _uiUpdateGate = new(); private static ElementUpdateAgent? _elementAgent; @@ -105,6 +107,8 @@ private static void ShowDiagnosticsOnFirstActivation(object snd, _WindowActivate /// private static async Task ReloadWithUpdatedTypes(HotReloadClientOperation? hrOp, Window window, Type[] updatedTypes) { + using var sequentialUiUpdateLock = await _uiUpdateGate.LockAsync(default); + var handlerActions = ElementAgent?.ElementHandlerActions; var uiUpdating = true;