Skip to content
Closed
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
29 changes: 29 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,34 @@
# Changelog

## 0.15.4

ARCH-1 step 4 (final). Fourth collaborator extracted from `TrayContext`. No user-visible changes. With this the audit's ARCH-1 incremental work is done.

Changed:

- **New `MountManager` class** (`src/MountManager.cs`) owns the rclone mount process lifecycle bits:
- `BuildMountArgs(profile, fullCache, rcCommonFlags, effectiveBandwidth)` — pure helper that assembles the full rclone `mount` argv with all the cache-mode, VFS, network-mode, RC, log, and bandwidth flags Pixelpipe has used since v0.5.x. Unit-testable without touching disk.
- `StartMountProcess(rclonePath, args, logJobWarn)` — `Process.Start` plus the v0.11.4 Job Object binding so the rclone child still dies with Pixelpipe.
- **`TrayContext.Mount.cs` delegates** argv assembly and the spawn. The dialog flow (rclone-missing, WinFsp-missing, remote-not-configured, drive-in-use, post-launch result) stays in `TrayContext` because it's UI-coupled.

## 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.

Changed:

- **New `SettingsStore` class** (`src/SettingsStore.cs`) owns the `settings.json` read / write / in-memory cache / atomic-write / timestamped backups / prune. Profile-specific shape logic (which keys land in each profile dict) stays in `TrayContext.Settings.cs`; `SettingsStore` is the layer below — give it the root dict, it handles disk.
- **`TrayContext.ReadSettingsRoot` / `WriteSettingsRoot` / `BackupSettingsFile` delegate** to a lazily-initialised `SettingsStore` field. Lock-protected cache (PERF-3 behavior) preserved.

## 0.15.1

ARCH-1 step 1 from the audit — first collaborator extracted from the `TrayContext` partial. No user-visible changes; internal refactor only.
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 value that is read and written across different threads (the UI thread and the background refresh worker thread) without synchronization. On 32-bit architectures, 64-bit reads and writes are not atomic, which can lead to torn reads. Additionally, without memory barriers, there is no guarantee of visibility or ordering.

Using a lock to synchronize access to all cached fields (_cachedRcloneAvailable, _cachedWinfspInstalled, and _cachedStampUtc) ensures thread safety and correctness across all platforms.

        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;
}
}
}
69 changes: 69 additions & 0 deletions src/MountManager.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
using System;
using System.Diagnostics;
using System.IO;

namespace Pixelpipe
{
// ARCH-1 step 4 (v0.15.4, audit): fourth and final collaborator
// extracted from the TrayContext partial. Owns just the rclone mount
// process-lifecycle bits: argument assembly + Process.Start + Job Object
// binding. The dialog flow (rclone-missing, WinFsp-missing, remote-not-
// configured, drive-in-use, post-launch result) stays in TrayContext
// because it's heavily UI-coupled.
//
// This is intentionally a thin extraction. The audit calls ARCH-1
// incremental; later releases can pull more orchestration in once the
// shape of "what doesn't need a TrayContext" stabilises.
internal sealed class MountManager
{
// Pure helper, tests cover it. Builds the rclone mount argv that
// Pixelpipe has used since v0.5.x with all the cache-mode, VFS,
// network-mode, RC, and bandwidth flags in one place. Caller passes
// RcCommonFlags + effective bandwidth so this class doesn't need to
// know about the auth token / per-profile bandwidth resolution.
internal static string BuildMountArgs(RemoteProfile p, bool fullCache, string rcCommonFlags, string effectiveBandwidth)
{
if (p == null) return "";
string cacheMode = fullCache ? "full" : "writes";
string args = "mount " + TrayContext.QuoteArg(TrayContext.NormalizeRemoteName(p.Remote))
+ " " + TrayContext.QuoteArg(TrayContext.NormalizeDriveLetter(p.DriveLetter))
+ " --links"
+ (String.Equals(p.MountMode, "network", StringComparison.OrdinalIgnoreCase) ? " --network-mode" : "")
+ " --vfs-cache-mode " + cacheMode
+ " --dir-cache-time 10m"
+ " --poll-interval 1m"
+ " --vfs-write-back 10s"
+ " --vfs-cache-max-age 6h"
+ " --vfs-cache-max-size 5G"
+ " --volname " + TrayContext.QuoteArg(p.Label)
+ " --rc " + (rcCommonFlags ?? "")
+ " --log-level INFO"
+ " --log-file " + TrayContext.QuoteArg(p.LogFile);
if (!String.IsNullOrEmpty(effectiveBandwidth)
&& !String.Equals(effectiveBandwidth, "off", StringComparison.OrdinalIgnoreCase))
{
args += " --bwlimit " + effectiveBandwidth;
}
return args;
}

// Spawns rclone mount and binds the child to the kill-on-job-close
// Job Object so it dies with Pixelpipe even on Task Manager kill /
// crash / sign-out. Returns the started Process; caller is
// responsible for the post-launch monitor (1900 ms wait + exit
// check + UI feedback).
internal static Process StartMountProcess(string rclonePath, string args, Action<string> logJobWarn)
{
ProcessStartInfo psi = new ProcessStartInfo();
psi.FileName = rclonePath;
psi.Arguments = args;
psi.UseShellExecute = false;
psi.CreateNoWindow = true;
psi.WindowStyle = ProcessWindowStyle.Hidden;
psi.WorkingDirectory = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile);
Process child = Process.Start(psi);
RcloneJob.TryAssign(child, logJobWarn);
return child;
}
}
}
35 changes: 6 additions & 29 deletions src/Pixelpipe.Mount.cs
Original file line number Diff line number Diff line change
Expand Up @@ -168,39 +168,16 @@ private void ContinueMountOnUiThread(RemoteProfile p, bool fullCache, bool rclon
p.FullCache = fullCache;
p.DesiredMounted = true;
p.LastError = "";
string cacheMode = fullCache ? "full" : "writes";
string args = "mount " + QuoteArg(NormalizeRemoteName(p.Remote)) + " " + QuoteArg(NormalizeDriveLetter(p.DriveLetter)) +
" --links" +
(String.Equals(p.MountMode, "network", StringComparison.OrdinalIgnoreCase) ? " --network-mode" : "") +
" --vfs-cache-mode " + cacheMode +
" --dir-cache-time 10m" +
" --poll-interval 1m" +
" --vfs-write-back 10s" +
" --vfs-cache-max-age 6h" +
" --vfs-cache-max-size 5G" +
" --volname " + QuoteArg(p.Label) +
" --rc " + RcCommonFlags(p.RcPort) +
" --log-level INFO" +
" --log-file " + QuoteArg(p.LogFile);

// ARCH-1 step 4 (v0.15.4): argv assembly and process spawn
// delegated to MountManager. The post-launch monitor stays here
// because it does UI work (BeginUi, MessageBox, ShowBalloon).
string effectiveBandwidth = EffectiveBandwidthFor(p);
if (!String.Equals(effectiveBandwidth, "off", StringComparison.OrdinalIgnoreCase)) args += " --bwlimit " + effectiveBandwidth;
string args = MountManager.BuildMountArgs(p, fullCache, RcCommonFlags(p.RcPort), effectiveBandwidth);

try
{
ProcessStartInfo psi = new ProcessStartInfo();
psi.FileName = rclonePath;
psi.Arguments = args;
psi.UseShellExecute = false;
psi.CreateNoWindow = true;
psi.WindowStyle = ProcessWindowStyle.Hidden;
psi.WorkingDirectory = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile);
p.MountProcess = Process.Start(psi);
// Bind the new rclone to our kill-on-job-close Job Object so
// it dies with Pixelpipe even if Pixelpipe is killed via Task
// Manager / crashes / etc. Best-effort; the orphan-scan path
// catches anything that slips through.
RcloneJob.TryAssign(p.MountProcess, delegate(string warn) { LogUiWarn("rclone job assign " + p.Label, warn); });
p.MountProcess = MountManager.StartMountProcess(rclonePath, args,
delegate(string warn) { LogUiWarn("rclone job assign " + p.Label, warn); });
p.StatusText = "mounting " + GetDriveRoot(p);
SaveProfiles();
RebuildMenu();
Expand Down
111 changes: 15 additions & 96 deletions src/Pixelpipe.Settings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -110,74 +110,26 @@ private void SaveProfiles()
catch (Exception ex) { LogUiIssue("save profiles", ex); }
}

// PERF-3 (v0.13.4): cache the parsed settings root in memory so
// each SaveSetting / SaveProfiles doesn't re-read + re-parse the
// file. Atomic-write durability is preserved (WriteSettingsRoot
// still goes through WriteAllTextAtomic), we just skip the read +
// JavaScriptSerializer round-trip per write. Lock-protected so
// concurrent writes from worker threads don't tear the dict.
private Dictionary<string, object> settingsRootCache;
private readonly object settingsRootCacheLock = new object();

private Dictionary<string, object> ReadSettingsRoot()
// ARCH-1 step 2 (v0.15.2): the cache / read / write / lock mechanics
// moved into SettingsStore. TrayContext holds a lazily-initialised
// reference and delegates. PERF-3 cache behavior unchanged.
private SettingsStore _settingsStore;
private SettingsStore SettingsBackend
{
lock (settingsRootCacheLock)
get
{
if (settingsRootCache != null) return settingsRootCache;
Dictionary<string, object> root;
if (TryReadSettingsRoot(settingsFile, out root))
{
settingsRootCache = root;
return settingsRootCache;
}
string backupFile = settingsFile + ".bak";
if (TryReadSettingsRoot(backupFile, out root))
if (_settingsStore == null)
{
LogUiWarn("read settings", "loaded backup settings file after primary file could not be read");
settingsRootCache = root;
return settingsRootCache;
_settingsStore = new SettingsStore(settingsFile, LogUiIssue,
delegate(string area, string msg) { LogUiWarn(area, msg); });
}
settingsRootCache = new Dictionary<string, object>(StringComparer.OrdinalIgnoreCase);
return settingsRootCache;
return _settingsStore;
}
}
Comment on lines +116 to 128
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 SettingsBackend property lazily instantiates _settingsStore without synchronization. Since SettingsBackend is accessed from both the UI thread and background worker threads (e.g., in SaveSetting), a race condition could result in multiple instances of SettingsStore being created concurrently.

Adding a lock ensures thread-safe lazy initialization.

        private readonly object _settingsStoreLock = new object();
        private SettingsStore _settingsStore;
        private SettingsStore SettingsBackend
        {
            get
            {
                lock (_settingsStoreLock)
                {
                    if (_settingsStore == null)
                    {
                        _settingsStore = new SettingsStore(settingsFile, LogUiIssue,
                            delegate(string area, string msg) { LogUiWarn(area, msg); });
                    }
                    return _settingsStore;
                }
            }
        }


private bool TryReadSettingsRoot(string path, out Dictionary<string, object> root)
{
root = null;
try
{
if (!File.Exists(path)) return false;
string json = File.ReadAllText(path, Encoding.UTF8);
JavaScriptSerializer js = new JavaScriptSerializer();
Dictionary<string, object> parsed = js.DeserializeObject(json) as Dictionary<string, object>;
if (parsed == null) return false;
root = new Dictionary<string, object>(parsed, StringComparer.OrdinalIgnoreCase);
return true;
}
catch (Exception ex) { LogUiIssue("read settings " + Path.GetFileName(path), ex); }
return false;
}
private Dictionary<string, object> ReadSettingsRoot() { return SettingsBackend.ReadRoot(); }

private void WriteSettingsRoot(Dictionary<string, object> root)
{
try
{
Directory.CreateDirectory(settingsDir);
string json;
lock (settingsRootCacheLock)
{
// Caller may have built `root` from scratch (vs mutating
// the existing cache); replace the cache reference so
// the next ReadSettingsRoot returns the latest values.
settingsRootCache = root;
JavaScriptSerializer js = new JavaScriptSerializer();
json = js.Serialize(root);
}
WriteAllTextAtomic(settingsFile, json, Encoding.UTF8);
}
catch (Exception ex) { LogUiIssue("write settings", ex); }
}
private void WriteSettingsRoot(Dictionary<string, object> root) { SettingsBackend.WriteRoot(root); }

// Pixelpipe already keeps the most recent settings.json as .bak via
// WriteAllTextAtomic (it's overwritten on every save). Before any
Expand All @@ -187,42 +139,9 @@ private void WriteSettingsRoot(Dictionary<string, object> root)
// recover a known-good state hours or days later. The directory is
// pruned to the last `BackupRetentionCount` files so it doesn't
// grow unbounded.
private const int BackupRetentionCount = 20;

internal string BackupsDir { get { return Path.Combine(settingsDir, "backups"); } }

private string BackupSettingsFile(string reason)
{
try
{
if (!File.Exists(settingsFile)) return null;
Directory.CreateDirectory(BackupsDir);
string stamp = DateTime.Now.ToString("yyyyMMdd-HHmmss");
string safeReason = SafeFileName(String.IsNullOrEmpty(reason) ? "manual" : reason);
string target = Path.Combine(BackupsDir, "settings-" + stamp + "-" + safeReason + ".json");
File.Copy(settingsFile, target, false);
PruneOldBackups();
LogUiWarn("settings backup", "wrote " + target);
return target;
}
catch (Exception ex) { LogUiIssue("settings backup " + reason, ex); return null; }
}

private void PruneOldBackups()
{
try
{
if (!Directory.Exists(BackupsDir)) return;
string[] files = Directory.GetFiles(BackupsDir, "settings-*.json");
if (files.Length <= BackupRetentionCount) return;
Array.Sort(files, delegate(string a, string b) { return File.GetCreationTimeUtc(b).CompareTo(File.GetCreationTimeUtc(a)); });
for (int i = BackupRetentionCount; i < files.Length; i++)
{
try { File.Delete(files[i]); } catch { }
}
}
catch (Exception ex) { LogUiIssue("prune backups", ex); }
}
// ARCH-1 step 2 (v0.15.2): backups + prune live in SettingsStore.
internal string BackupsDir { get { return SettingsBackend.BackupsDir; } }
private string BackupSettingsFile(string reason) { return SettingsBackend.Backup(reason); }

// Tools / diagnostics menu hook.
private void OpenSettingsBackupsFolder()
Expand Down
Loading
Loading