Skip to content

Commit 26fc29b

Browse files
committed
refactor: move WorkingCopy.SetData to UIThread to avoid data-race condition (#2203)
Signed-off-by: leo <longshuang@msn.cn>
1 parent fd667f3 commit 26fc29b

File tree

2 files changed

+57
-68
lines changed

2 files changed

+57
-68
lines changed

src/ViewModels/Repository.cs

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1326,29 +1326,15 @@ public void RefreshWorkingCopyChanges()
13261326
.GetResultAsync()
13271327
.ConfigureAwait(false);
13281328

1329-
if (_workingCopy == null || token.IsCancellationRequested)
1330-
return;
1331-
13321329
changes.Sort((l, r) => Models.NumericSort.Compare(l.Path, r.Path));
1333-
_workingCopy.SetData(changes, token);
1334-
1335-
var hasModified = false;
1336-
foreach (var c in changes)
1337-
{
1338-
if (c.Index == Models.ChangeState.Added || c.WorkTree == Models.ChangeState.Untracked)
1339-
continue;
1340-
1341-
hasModified = true;
1342-
break;
1343-
}
13441330

13451331
Dispatcher.UIThread.Invoke(() =>
13461332
{
13471333
if (token.IsCancellationRequested)
13481334
return;
13491335

1336+
_workingCopy.SetData(changes);
13501337
LocalChangesCount = changes.Count;
1351-
_canCheckoutDirectly = !hasModified;
13521338
OnPropertyChanged(nameof(InProgressContext));
13531339
GetOwnerPage()?.ChangeDirtyState(Models.DirtyState.HasLocalChanges, changes.Count == 0);
13541340
});
@@ -1422,7 +1408,7 @@ public async Task CheckoutBranchAsync(Models.Branch branch)
14221408

14231409
if (branch.IsLocal)
14241410
{
1425-
if (_canCheckoutDirectly)
1411+
if (_workingCopy is { CanSwitchBranchDirectly: true })
14261412
await ShowAndStartPopupAsync(new Checkout(this, branch));
14271413
else
14281414
ShowPopup(new Checkout(this, branch));
@@ -1979,7 +1965,6 @@ private async Task AutoFetchOnUIThread()
19791965
private List<Models.Submodule> _submodules = [];
19801966
private object _visibleSubmodules = null;
19811967
private string _navigateToCommitDelayed = string.Empty;
1982-
private bool _canCheckoutDirectly = false;
19831968

19841969
private bool _isAutoFetching = false;
19851970
private Timer _autoFetchTimer = null;

src/ViewModels/WorkingCopy.cs

Lines changed: 55 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
using System;
22
using System.Collections.Generic;
33
using System.IO;
4-
using System.Threading;
54
using System.Threading.Tasks;
6-
7-
using Avalonia.Threading;
85
using CommunityToolkit.Mvvm.ComponentModel;
96

107
namespace SourceGit.ViewModels
@@ -41,6 +38,12 @@ public bool HasUnsolvedConflicts
4138
set => SetProperty(ref _hasUnsolvedConflicts, value);
4239
}
4340

41+
public bool CanSwitchBranchDirectly
42+
{
43+
get;
44+
set;
45+
} = false;
46+
4447
public InProgressContext InProgressContext
4548
{
4649
get => _inProgressContext;
@@ -256,64 +259,67 @@ public void Dispose()
256259
_commitMessage = string.Empty;
257260
}
258261

259-
public void SetData(List<Models.Change> changes, CancellationToken cancellationToken)
262+
public void SetData(List<Models.Change> changes)
260263
{
261264
if (!IsChanged(_cached, changes))
262265
{
263-
// Just force refresh selected changes.
264-
Dispatcher.UIThread.Invoke(() =>
265-
{
266-
if (cancellationToken.IsCancellationRequested)
267-
return;
268-
269-
HasUnsolvedConflicts = _cached.Find(x => x.IsConflicted) != null;
270-
UpdateInProgressState();
271-
UpdateDetail();
272-
});
273-
266+
HasUnsolvedConflicts = _cached.Find(x => x.IsConflicted) != null;
267+
UpdateInProgressState();
268+
UpdateDetail();
274269
return;
275270
}
276271

277272
var lastSelectedUnstaged = new HashSet<string>();
278-
var lastSelectedStaged = new HashSet<string>();
279273
if (_selectedUnstaged is { Count: > 0 })
280274
{
281275
foreach (var c in _selectedUnstaged)
282276
lastSelectedUnstaged.Add(c.Path);
283277
}
284-
else if (_selectedStaged is { Count: > 0 })
285-
{
286-
foreach (var c in _selectedStaged)
287-
lastSelectedStaged.Add(c.Path);
288-
}
289278

290279
var unstaged = new List<Models.Change>();
280+
var visibleUnstaged = new List<Models.Change>();
281+
var selectedUnstaged = new List<Models.Change>();
282+
var noFilter = string.IsNullOrEmpty(_filter);
291283
var hasConflict = false;
284+
var canSwitchDirectly = true;
292285
foreach (var c in changes)
293286
{
294287
if (c.WorkTree != Models.ChangeState.None)
295288
{
296289
unstaged.Add(c);
297290
hasConflict |= c.IsConflicted;
291+
292+
if (noFilter || c.Path.Contains(_filter, StringComparison.OrdinalIgnoreCase))
293+
{
294+
visibleUnstaged.Add(c);
295+
if (lastSelectedUnstaged.Contains(c.Path))
296+
selectedUnstaged.Add(c);
297+
}
298298
}
299-
}
300299

301-
var visibleUnstaged = GetVisibleChanges(unstaged);
302-
var selectedUnstaged = new List<Models.Change>();
303-
foreach (var c in visibleUnstaged)
304-
{
305-
if (lastSelectedUnstaged.Contains(c.Path))
306-
selectedUnstaged.Add(c);
300+
if (!canSwitchDirectly)
301+
continue;
302+
303+
if (c.WorkTree == Models.ChangeState.Untracked || c.Index == Models.ChangeState.Added)
304+
continue;
305+
306+
canSwitchDirectly = false;
307307
}
308308

309309
var staged = GetStagedChanges(changes);
310-
311310
var visibleStaged = GetVisibleChanges(staged);
312311
var selectedStaged = new List<Models.Change>();
313-
foreach (var c in visibleStaged)
312+
if (_selectedStaged is { Count: > 0 })
314313
{
315-
if (lastSelectedStaged.Contains(c.Path))
316-
selectedStaged.Add(c);
314+
var set = new HashSet<string>();
315+
foreach (var c in _selectedStaged)
316+
set.Add(c.Path);
317+
318+
foreach (var c in visibleStaged)
319+
{
320+
if (set.Contains(c.Path))
321+
selectedStaged.Add(c);
322+
}
317323
}
318324

319325
if (selectedUnstaged.Count == 0 && selectedStaged.Count == 0 && hasConflict)
@@ -322,25 +328,20 @@ public void SetData(List<Models.Change> changes, CancellationToken cancellationT
322328
selectedUnstaged.Add(firstConflict);
323329
}
324330

325-
Dispatcher.UIThread.Invoke(() =>
326-
{
327-
if (cancellationToken.IsCancellationRequested)
328-
return;
329-
330-
_isLoadingData = true;
331-
_cached = changes;
332-
HasUnsolvedConflicts = hasConflict;
333-
VisibleUnstaged = visibleUnstaged;
334-
VisibleStaged = visibleStaged;
335-
Unstaged = unstaged;
336-
Staged = staged;
337-
SelectedUnstaged = selectedUnstaged;
338-
SelectedStaged = selectedStaged;
339-
_isLoadingData = false;
331+
_isLoadingData = true;
332+
_cached = changes;
333+
HasUnsolvedConflicts = hasConflict;
334+
CanSwitchBranchDirectly = canSwitchDirectly;
335+
VisibleUnstaged = visibleUnstaged;
336+
VisibleStaged = visibleStaged;
337+
Unstaged = unstaged;
338+
Staged = staged;
339+
SelectedUnstaged = selectedUnstaged;
340+
SelectedStaged = selectedStaged;
341+
_isLoadingData = false;
340342

341-
UpdateInProgressState();
342-
UpdateDetail();
343-
});
343+
UpdateInProgressState();
344+
UpdateDetail();
344345
}
345346

346347
public async Task StageChangesAsync(List<Models.Change> changes, Models.Change next)
@@ -671,8 +672,11 @@ public async Task CommitAsync(bool autoStage, bool autoPush)
671672

672673
if (succ)
673674
{
675+
// Do not use property `UseAmend` but manually trigger property changed to avoid refreshing staged changes here.
676+
_useAmend = false;
677+
OnPropertyChanged(nameof(UseAmend));
678+
674679
CommitMessage = string.Empty;
675-
UseAmend = false;
676680
if (autoPush && _repo.Remotes.Count > 0)
677681
{
678682
Models.Branch pushBranch = null;

0 commit comments

Comments
 (0)