Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
# Changelog

## 0.15.3

ARCH-1 step 3. Third collaborator extracted from `TrayContext`. No user-visible changes.

Changed:

- **New `DependencyProbe` class** (`src/DependencyProbe.cs`) owns the cached `RcloneAvailable` / `WinFspInstalled` booleans plus their TTL stamp and the synchronous probes themselves (the slow `File.Exists` + `spawn rclone version` + registry lookups). Constructor takes a `Func<string>` rclone path provider and a log callback.
- **`TrayContext.Setup.cs` delegates** the cached-state and probe methods. The async refresh worker (`RefreshDependencyStatusAsync`) stays in `TrayContext` because it composes results into the setup-status line and marshals back via `BeginUi`; it just hands the actual probing off to `DependencyProbe.PublishProbeResults`.

## 0.15.2

ARCH-1 step 2. Second collaborator extracted from `TrayContext`. No user-visible changes.
Expand Down
79 changes: 79 additions & 0 deletions src/DependencyProbe.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
using System;
using System.IO;
using Microsoft.Win32;

namespace Pixelpipe
{
// ARCH-1 step 3 (v0.15.3, audit): third collaborator extracted from
// the TrayContext partial. Owns the cached rclone / WinFsp availability
// state plus the synchronous probes themselves.
//
// The TTL + UI-thread-friendly cached accessors stay where they were
// semantically — UI callers ask "is rclone available?" and get back
// whatever the last refresh wrote, never touching disk on the UI thread.
// The async refresh worker (RefreshDependencyStatusAsync) stays in
// TrayContext for now because it composes results into the setup-status
// line; it just hands off the actual probe calls to this class.
internal sealed class DependencyProbe
{
public const int CacheTtlSeconds = 30;

private readonly Func<string> _rclonePathProvider;
private readonly Action<string, Exception> _logIssue;
private volatile bool _cachedRcloneAvailable;
private volatile bool _cachedWinfspInstalled;
private DateTime _cachedStampUtc = DateTime.MinValue;

public DependencyProbe(Func<string> rclonePathProvider, Action<string, Exception> logIssue)
{
_rclonePathProvider = rclonePathProvider;
_logIssue = logIssue;
}

public bool RcloneAvailable { get { return _cachedRcloneAvailable; } }
public bool WinFspInstalled { get { return _cachedWinfspInstalled; } }
public DateTime LastProbeUtc { get { return _cachedStampUtc; } }
public bool IsStale { get { return (DateTime.UtcNow - _cachedStampUtc).TotalSeconds >= CacheTtlSeconds; } }

// Called by the async refresh worker after the slow probes complete.
public void PublishProbeResults(bool rcloneAvailable, bool winfspInstalled)
{
_cachedRcloneAvailable = rcloneAvailable;
_cachedWinfspInstalled = winfspInstalled;
_cachedStampUtc = DateTime.UtcNow;
}
Comment on lines +23 to +44
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The _cachedStampUtc field is a 64-bit DateTime struct, and volatile cannot be applied to it in C#. Since IsStale and PublishProbeResults are accessed concurrently across the UI thread and background threads, there is a risk of memory reordering or stale reads.

To ensure complete thread safety, atomicity, and proper memory barriers without relying on complex volatile ordering, we can use a simple lock to protect the cached fields and properties. Since these are read infrequently (on UI refresh ticks) and written even less frequently (every 30 seconds), lock contention will be virtually zero.

        private readonly object _lock = new object();
        private bool _cachedRcloneAvailable;
        private bool _cachedWinfspInstalled;
        private DateTime _cachedStampUtc = DateTime.MinValue;

        public DependencyProbe(Func<string> rclonePathProvider, Action<string, Exception> logIssue)
        {
            _rclonePathProvider = rclonePathProvider;
            _logIssue = logIssue;
        }

        public bool RcloneAvailable { get { lock (_lock) return _cachedRcloneAvailable; } }
        public bool WinFspInstalled { get { lock (_lock) return _cachedWinfspInstalled; } }
        public DateTime LastProbeUtc { get { lock (_lock) return _cachedStampUtc; } }
        public bool IsStale
        {
            get
            {
                lock (_lock)
                {
                    return (DateTime.UtcNow - _cachedStampUtc).TotalSeconds >= CacheTtlSeconds;
                }
            }
        }

        // Called by the async refresh worker after the slow probes complete.
        public void PublishProbeResults(bool rcloneAvailable, bool winfspInstalled)
        {
            lock (_lock)
            {
                _cachedRcloneAvailable = rcloneAvailable;
                _cachedWinfspInstalled = winfspInstalled;
                _cachedStampUtc = DateTime.UtcNow;
            }
        }


// Synchronous rclone probe. Tries the resolved path first; if that
// file isn't present, falls back to spawning `rclone.exe version`
// (PATH resolution). Returns true if either path responds.
public bool ProbeRcloneSync(Func<string, int, string> runProcessCapture)
{
try
{
string resolved = _rclonePathProvider == null ? null : _rclonePathProvider();
if (!String.IsNullOrEmpty(resolved) && File.Exists(resolved)) return true;
if (runProcessCapture == null) return false;
string version = runProcessCapture("rclone.exe", 3000);
return !String.IsNullOrEmpty(version) && version.IndexOf("rclone", StringComparison.OrdinalIgnoreCase) >= 0;
}
catch (Exception ex) { if (_logIssue != null) _logIssue("dep probe rclone", ex); return false; }
}

// Synchronous WinFsp probe. Two on-disk dll locations + two registry
// keys; any positive signal is enough.
public bool ProbeWinFspSync()
{
try
{
string pf86 = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFilesX86);
string pf = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles);
if (File.Exists(Path.Combine(pf86, "WinFsp", "bin", "winfsp-x64.dll"))) return true;
if (File.Exists(Path.Combine(pf, "WinFsp", "bin", "winfsp-x64.dll"))) return true;
using (RegistryKey k1 = Registry.LocalMachine.OpenSubKey(@"SOFTWARE\WinFsp")) { if (k1 != null) return true; }
using (RegistryKey k2 = Registry.LocalMachine.OpenSubKey(@"SOFTWARE\WOW6432Node\WinFsp")) { if (k2 != null) return true; }
}
catch (Exception ex) { if (_logIssue != null) _logIssue("dep probe winfsp", ex); }
return false;
}
}
}
83 changes: 28 additions & 55 deletions src/Pixelpipe.Setup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,70 +43,45 @@ private void RunFirstLaunchSetup(bool manual)
}
}

// PERF-1 (v0.13.1): both RcloneAvailable and WinFspInstalled used
// to be called repeatedly from UI-thread paths (ApplyLiveState,
// UpdateMenuLiveState) every refresh tick. RcloneAvailable on a
// cold path could spawn `rclone.exe version` with a 3 s wait —
// multiple times per ~7 s refresh = visible stutter. We now cache
// the booleans with a TTL; RefreshDependencyStatusAsync (which
// already runs off-thread) is the one place that does the real
// probes and writes the cached fields. UI callers read the cached
// values via the public methods below, which never touch disk.
private const int DependencyCacheTtlSeconds = 30;
private volatile bool cachedRcloneAvailable;
private volatile bool cachedWinfspInstalled;
private DateTime cachedDependencyStampUtc = DateTime.MinValue;

private bool RcloneAvailable()
// ARCH-1 step 3 (v0.15.3): cached dependency state + sync probes
// moved into DependencyProbe. The async refresh worker stays here
// because it composes results into the setup-status line and posts
// back via BeginUi.
private DependencyProbe _depProbe;
private DependencyProbe Deps
{
EnsureDependencyCacheFresh();
return cachedRcloneAvailable;
get
{
if (_depProbe == null)
{
_depProbe = new DependencyProbe(
delegate { rclonePath = FindRclonePath(); return rclonePath; },
LogUiIssue);
}
return _depProbe;
}
}
Comment on lines +50 to 63
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The Deps property lazily initializes _depProbe without any thread-safety guards. Since Deps is accessed concurrently from both the UI thread (e.g., via RcloneAvailable()) and background threads (via ProbeRcloneAvailableSync() on a ThreadPool worker), there is a race condition where multiple instances of DependencyProbe could be created, or a thread could read a partially-constructed object.

To prevent this, we should use a lock or double-check locking to ensure thread-safe lazy initialization.

        private readonly object _depProbeLock = new object();
        private DependencyProbe _depProbe;
        private DependencyProbe Deps
        {
            get
            {
                if (_depProbe == null)
                {
                    lock (_depProbeLock)
                    {
                        if (_depProbe == null)
                        {
                            _depProbe = new DependencyProbe(
                                delegate { rclonePath = FindRclonePath(); return rclonePath; },
                                LogUiIssue);
                        }
                    }
                }
                return _depProbe;
            }
        }


private bool WinFspInstalled()
private bool RcloneAvailable()
{
EnsureDependencyCacheFresh();
return cachedWinfspInstalled;
if (Deps.IsStale) RefreshDependencyStatusAsync(false);
return Deps.RcloneAvailable;
}

// Lazy first-call probe: if the cache is empty / stale and we're on
// a UI thread, kick the async refresh and return whatever we have
// (false on cold start). UI updates within one refresh cycle.
private void EnsureDependencyCacheFresh()
private bool WinFspInstalled()
{
if ((DateTime.UtcNow - cachedDependencyStampUtc).TotalSeconds < DependencyCacheTtlSeconds) return;
RefreshDependencyStatusAsync(false);
if (Deps.IsStale) RefreshDependencyStatusAsync(false);
return Deps.WinFspInstalled;
}

// Synchronous probes — the real File.Exists / spawn-rclone / registry
// work — called only from the async refresh worker. Anything UI-bound
// must go through RcloneAvailable() / WinFspInstalled() above.
// Compatibility shims used by the refresh worker; both just forward
// to DependencyProbe.
private bool ProbeRcloneAvailableSync()
{
try
{
rclonePath = FindRclonePath();
if (File.Exists(rclonePath)) return true;
string version = RunProcessCapture("rclone.exe", "version", 3000);
return version.IndexOf("rclone", StringComparison.OrdinalIgnoreCase) >= 0;
}
catch { return false; }
return Deps.ProbeRcloneSync(delegate(string exe, int timeoutMs) { return RunProcessCapture(exe, "version", timeoutMs); });
}

private bool ProbeWinFspInstalledSync()
{
try
{
string pf86 = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFilesX86);
string pf = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles);
if (File.Exists(Path.Combine(pf86, "WinFsp", "bin", "winfsp-x64.dll"))) return true;
if (File.Exists(Path.Combine(pf, "WinFsp", "bin", "winfsp-x64.dll"))) return true;
using (RegistryKey k1 = Registry.LocalMachine.OpenSubKey(@"SOFTWARE\WinFsp")) { if (k1 != null) return true; }
using (RegistryKey k2 = Registry.LocalMachine.OpenSubKey(@"SOFTWARE\WOW6432Node\WinFsp")) { if (k2 != null) return true; }
}
catch { }
return false;
}
private bool ProbeWinFspInstalledSync() { return Deps.ProbeWinFspSync(); }

private bool AnyRemoteConfigured()
{
Expand Down Expand Up @@ -140,7 +115,7 @@ private string[] GetCachedRcloneRemotes(bool force)
// Cheap cached lookup — never triggers a fresh disk probe; if
// the dependency cache hasn't been seeded yet this returns false
// and we keep the existing remotes cache rather than spinning.
if (!cachedRcloneAvailable) return cachedRcloneRemotes;
if (!Deps.RcloneAvailable) return cachedRcloneRemotes;
string output;
try { output = RunRcloneCapture("listremotes", 6000); }
catch (Exception ex) { LogUiIssue("listremotes", ex); return cachedRcloneRemotes; }
Expand Down Expand Up @@ -183,9 +158,7 @@ private void RefreshDependencyStatusAsync(bool force)
catch (Exception ex) { LogUiIssue("dependency status", ex); text = setupStatusText; }
BeginUi(delegate
{
cachedRcloneAvailable = rcloneProbe;
cachedWinfspInstalled = winfspProbe;
cachedDependencyStampUtc = DateTime.UtcNow;
Deps.PublishProbeResults(rcloneProbe, winfspProbe);
setupStatusText = text;
lastDependencyRefreshUtc = DateTime.UtcNow;
Interlocked.Exchange(ref dependencyRefreshingFlag, 0);
Expand Down
Loading