Skip to content

Commit

Permalink
fix: Fix UpdateFileAsync requesting HR before VS detects file change
Browse files Browse the repository at this point in the history
  • Loading branch information
dr1rrb committed Oct 1, 2024
1 parent 0e6eeab commit 380535e
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -219,6 +219,7 @@ private class HotReloadServerOperation
private ImmutableHashSet<string> _filePaths;
private int /* HotReloadResult */ _result = -1;
private CancellationTokenSource? _deferredCompletion;
private int _noChangesRetry;

public long Id { get; } = Interlocked.Increment(ref _count);

Expand Down Expand Up @@ -290,6 +291,12 @@ public bool TryMerge(ImmutableHashSet<string> filePaths)
}
}

/// <summary>
/// Configure a simple auto-retry strategy if no changes are detected.
/// </summary>
public void EnableAutoRetryIfNoChanges()
=> _noChangesRetry = 1;

/// <summary>
/// As errors might get a bit after the complete from the IDE, we can defer the completion of the operation.
/// </summary>
Expand All @@ -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
Expand Down Expand Up @@ -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);
}

Expand All @@ -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))
{
Expand All @@ -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}].");
Expand All @@ -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))
{
Expand All @@ -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))
{
Expand All @@ -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<bool> RequestHotReloadToIde(long sequenceId)
Expand All @@ -545,6 +599,7 @@ private async Task<bool> 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 };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public record struct UpdateRequest(
/// The max delay to wait for the local application to process a hot-reload delta.
/// </summary>
/// <remarks>This includes the time to apply the delta locally and then to run all local handlers.</remarks>
public TimeSpan LocalHotReloadTimeout { get; set; } = TimeSpan.FromSeconds(3);
public TimeSpan LocalHotReloadTimeout { get; set; } = TimeSpan.FromSeconds(5);

public UpdateRequest WithExtendedTimeouts(float? factor = null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -105,6 +107,8 @@ private static void ShowDiagnosticsOnFirstActivation(object snd, _WindowActivate
/// </summary>
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;
Expand Down

0 comments on commit 380535e

Please sign in to comment.