DYN-9707: Harden graph locking against corrupt lock files and improve Save As safety

src/DynamoCore/Graph/Workspaces/Locking/GraphLockFile.cs

@@ -8,6 +8,17 @@
 
 namespace Dynamo.Graph.Workspaces.Locking
 {
+    /// <summary>
+    /// Describes the outcome of a <see cref="GraphLockFile.TryRead"/> call.
+    /// </summary>
+    internal enum GraphLockReadResult
+    {
+        NotFound,
+        Ok,
+        Corrupt,
+        TransientFailure
+    }
+
     /// <summary>
     /// Provides file-system operations for graph lock sidecar files.
     /// </summary>
@@ -24,6 +35,12 @@ internal static class GraphLockFile
             TypeNameHandling = TypeNameHandling.None
         });
 
+        private static bool IsValidLockInfo(GraphLockInfo info) =>
+            info != null
+            && info.SessionId != Guid.Empty
+            && !string.IsNullOrEmpty(info.GraphPath)
+            && info.ProcessId > 0;
+
         /// <summary>
         /// Gets the sidecar lock path for a graph file.
         /// </summary>
@@ -65,10 +82,13 @@ internal static bool TryCreateNewLockFile(string sidecarPath, GraphLockInfo info
         /// <param name="sidecarPath">The sidecar lock file path.</param>
         /// <param name="info">The parsed lock metadata when reading succeeds.</param>
         /// <returns>True if metadata was read; otherwise false.</returns>
-        internal static bool TryRead(string sidecarPath, out GraphLockInfo info)
+        internal static GraphLockReadResult TryRead(string sidecarPath, out GraphLockInfo info)
         {
             info = null;
 
+            if (!File.Exists(sidecarPath))
+                return GraphLockReadResult.NotFound;
+
             const int maxAttempts = 2;
             for (var attempt = 0; attempt < maxAttempts; attempt++)
             {
@@ -81,29 +101,25 @@ internal static bool TryRead(string sidecarPath, out GraphLockInfo info)
                         info = Serializer.Deserialize<GraphLockInfo>(jsonReader);
                     }
 
-                    return info != null;
+                    return IsValidLockInfo(info) ? GraphLockReadResult.Ok : GraphLockReadResult.Corrupt;
                 }
-                catch (Exception ex) when (ex is IOException || ex is JsonException)
+                catch (Exception ex) when (ex is JsonException)
                 {
-                    if (attempt == maxAttempts - 1)
-                    {
-                        return false;
-                    }
-
-                    // Wait for 50 milliseconds before retrying
-                    Thread.Sleep(50);
+                    return GraphLockReadResult.Corrupt;
                 }
-                catch (UnauthorizedAccessException)
+                catch (Exception ex) when (ex is IOException)
                 {
-                    return false;
+                    if (attempt == maxAttempts - 1)
+                        return GraphLockReadResult.TransientFailure;
+                    Thread.Sleep(50);
                 }
-                catch (SecurityException)
+                catch (Exception ex) when (ex is UnauthorizedAccessException || ex is SecurityException)
                 {
-                    return false;
+                    return GraphLockReadResult.TransientFailure;
                 }
             }
 
-            return false;
+            return GraphLockReadResult.TransientFailure;
         }
 
         /// <summary>

src/DynamoCore/Graph/Workspaces/Locking/GraphLockManager.cs

@@ -15,8 +15,9 @@ namespace Dynamo.Graph.Workspaces.Locking
     /// </summary>
     internal sealed class GraphLockManager : IDisposable
     {
-        internal const int DefaultHeartbeatMilliseconds = 30000;
+        internal const int DefaultHeartbeatMilliseconds = 15000;
         private const int StaleFactor = 3;
+        private const int MaxConsecutiveReadFailures = 3;
 
         private readonly DynamoModel dynamoModel;
         private readonly StringComparer pathComparer;
@@ -37,6 +38,7 @@ private sealed class OwnedLock
             internal string SidecarPath { get; set; }
             internal GraphLockInfo Info { get; set; }
             internal WorkspaceModel Workspace { get; set; }
+            internal int ConsecutiveReadFailures { get; set; }
         }
 
         /// <summary>
@@ -192,22 +194,28 @@ private GraphLockAcquireResult AcquireLockInternal(string normalizedPath, bool a
                     }
 
                     // A lock file already exists: read it to find out who owns it
-                    GraphLockInfo existingLock;
-                    var readable = GraphLockFile.TryRead(sidecarPath, out existingLock);
+                    var readResult = GraphLockFile.TryRead(sidecarPath, out var existingLock);
 
-                    var lockResult = TryAcquireReadableLock(normalizedPath, sidecarPath, info, workspace, readable, existingLock);
+                    if (readResult == GraphLockReadResult.NotFound)
+                    {
+                        // The sidecar was deleted between TryCreateNewLockFile failing and TryRead.
+                        // Retry the loop — TryCreateNewLockFile will succeed on the next attempt.
+                        continue;
+                    }
+
+                    var lockResult = TryAcquireReadableLock(normalizedPath, sidecarPath, info, workspace, readResult, existingLock);
                     if (lockResult != null)
                     {
                         return lockResult;
                     }
 
-                    // A live (or currently unreadable) lock owns the path.
+                    // ExistingLock is Ok and live — a real conflict with another session.
                     return ResolveLockConflict(normalizedPath, allowPromptUI, workspace, existingLock);
                 }
                 catch (Exception ex) when (ex is UnauthorizedAccessException || ex is SecurityException || ex is IOException)
                 {
                     dynamoModel.Logger?.Log("GraphLock unavailable: " + ex.Message);
-                }                
+                }
             }
 
             return GraphLockAcquireResult.Unavailable(normalizedPath);
@@ -218,26 +226,34 @@ private GraphLockAcquireResult TryAcquireReadableLock(
             string sidecarPath,
             GraphLockInfo info,
             WorkspaceModel workspace,
-            bool readable,
+            GraphLockReadResult readResult,
             GraphLockInfo existingLock)
         {
-            if (!readable)
+            if (readResult == GraphLockReadResult.Corrupt)
             {
-                return null;
+                // Definitively unreadable — not evidence of a live owner.
+                // Safe to overwrite and take ownership.
+                GraphLockFile.WriteHeartbeat(sidecarPath, info);
+                RegisterOwnedLock(normalizedPath, sidecarPath, info, workspace);
+                return GraphLockAcquireResult.Acquired(normalizedPath);
+            }
+
+            if (readResult == GraphLockReadResult.TransientFailure)
+            {
+                // Could not read due to IO/permissions — a live owner may exist.
+                // Do not steal the lock. Redirect to copy to be safe.
+                dynamoModel.Logger?.Log( "GraphLock: could not read sidecar due to transient failure, redirecting to copy: " + sidecarPath);
+                return GraphLockAcquireResult.Unavailable(normalizedPath);
             }
 
-            // It is our own lock (same machine + process): reuse it
             if (IsOwnedByThisSession(existingLock))
             {
                 RegisterOwnedLock(normalizedPath, sidecarPath, existingLock, workspace);
                 return GraphLockAcquireResult.Acquired(normalizedPath);
             }
 
             // The lock is expired (no recent heartbeat) or owned by a process on this machine
-            // that is no longer running. In these cases the previous owner is gone, so we
-            // silently take the lock over. A present but unreadable lock is not reclaimed here,
-            // it is treated as a real conflict below, so that a transient read failure cannot
-            // make us steal a lock that a live instance still owns.
+            // that is no longer running. Silently take ownership.
             if (IsStale(existingLock) || IsDeadLocalProcess(existingLock))
             {
                 GraphLockFile.WriteHeartbeat(sidecarPath, info);
@@ -297,13 +313,13 @@ private GraphLockAcquireResult CreateAndOpenCopy( string sourcePath, string save
                 else
                 {
                     GraphLockInfo saveAsLock;
-                    var readable = GraphLockFile.TryRead(sidecarPath, out saveAsLock);
-                    if (readable && IsOwnedByThisSession(saveAsLock))
+                    var saveAsReadResult = GraphLockFile.TryRead(sidecarPath, out saveAsLock);
+                    if (saveAsReadResult == GraphLockReadResult.Ok && IsOwnedByThisSession(saveAsLock))
                     {
                         info = saveAsLock;
                         ownsSaveAsLock = true;
                     }
-                    else if (!readable || IsStale(saveAsLock) || IsDeadLocalProcess(saveAsLock))
+                    else if (saveAsReadResult == GraphLockReadResult.Corrupt || IsStale(saveAsLock) || IsDeadLocalProcess(saveAsLock))
                     {
                         GraphLockFile.WriteHeartbeat(sidecarPath, info);
                         ownsSaveAsLock = true;
@@ -341,17 +357,42 @@ private void RegisterOwnedLock(string normalizedPath, string sidecarPath, GraphL
             };
         }
 
-        private void RefreshHeartbeats(object state)
+        internal void RefreshHeartbeats(object state = null)
         {
             foreach (var pair in locks.ToList())
             {
                 var owned = pair.Value;
                 try
                 {
                     GraphLockInfo current;
-                    if (!GraphLockFile.TryRead(owned.SidecarPath, out current) ||
-                        current.SessionId != owned.Info.SessionId)
+                    if (GraphLockFile.TryRead(owned.SidecarPath, out current) != GraphLockReadResult.Ok)
+                    {
+                        // Transient IO failure (antivirus, NAS hiccup) — don't drop the lock immediately.
+                        // Only abandon after MaxConsecutiveReadFailures consecutive misses.                        
+                        owned.ConsecutiveReadFailures++;
+
+                        if (owned.ConsecutiveReadFailures >= MaxConsecutiveReadFailures)
+                        {
+                            dynamoModel.Logger?.Log(
+                                $"GraphLock heartbeat read failed {MaxConsecutiveReadFailures} consecutive times, " +
+                                $"dropping lock: {owned.SidecarPath}");
+                            locks.TryRemove(pair.Key, out _);
+                        }
+                        else
+                        {
+                            dynamoModel.Logger?.Log(
+                                $"GraphLock heartbeat read failed (attempt {owned.ConsecutiveReadFailures}), " +
+                                $"will retry: {owned.SidecarPath}");
+                        }
+                        continue;
+                    }
+
+                    // Successful read — reset the failure counter
+                    owned.ConsecutiveReadFailures = 0;
+
+                    if (current.SessionId != owned.Info.SessionId)
                     {
+                        // Lock was genuinely stolen by another instance — stop tracking
                         locks.TryRemove(pair.Key, out _);
                         continue;
                     }
@@ -371,7 +412,7 @@ private void ReleaseOwnedLock(string normalizedPath, OwnedLock owned)
             try
             {
                 GraphLockInfo current;
-                if (GraphLockFile.TryRead(owned.SidecarPath, out current) &&
+                if (GraphLockFile.TryRead(owned.SidecarPath, out current) == GraphLockReadResult.Ok &&
                     current.SessionId == owned.Info.SessionId)
                 {
                     GraphLockFile.TryDelete(owned.SidecarPath);

src/DynamoCoreWpf/Properties/Resources.en-US.resx

@@ -4313,4 +4313,15 @@ To avoid data loss or corruption, please choose one of the following actions:
   <data name="GraphLockSaveAsButton" xml:space="preserve">
     <value>Save As</value>
   </data>
+  <data name="GraphLockSaveAsSameFileMessage" xml:space="preserve">
+    <value>This file is currently open in another instance of Dynamo.
+To avoid data loss or corruption, please choose a different file name.</value>
+  </data>
+  <data name="ConfirmReplaceFileMessage" xml:space="preserve">
+    <value> {0} already exists.
+Do you want to replace it?</value>
+  </data>
+  <data name="ConfirmReplaceFileTitle" xml:space="preserve">
+    <value>Confirm Save As</value>
+  </data>
 </root>

src/DynamoCoreWpf/Properties/Resources.resx

@@ -4300,4 +4300,15 @@ To avoid data loss or corruption, please choose one of the following actions:
   <data name="GraphLockSaveAsButton" xml:space="preserve">
     <value>Save As</value>
   </data>
+  <data name="GraphLockSaveAsSameFileMessage" xml:space="preserve">
+    <value>This file is currently open in another instance of Dynamo.
+To avoid data loss or corruption, please choose a different file name.</value>
+  </data>
+  <data name="ConfirmReplaceFileMessage" xml:space="preserve">
+    <value> {0} already exists.
+Do you want to replace it?</value>
+  </data>
+  <data name="ConfirmReplaceFileTitle" xml:space="preserve">
+    <value>Confirm Save As</value>
+  </data>
 </root>

src/DynamoCoreWpf/PublicAPI.Unshipped.txt

@@ -33,6 +33,9 @@ Dynamo.ViewModels.PreferencesViewModel.AutoSyncDocumentBrowserIsChecked.get -> b
 Dynamo.ViewModels.PreferencesViewModel.AutoSyncDocumentBrowserIsChecked.set -> void
 Dynamo.ViewModels.PreferencesViewModel.ShowPythonAutoMigrationNotificationIsChecked.get -> bool
 Dynamo.ViewModels.PreferencesViewModel.ShowPythonAutoMigrationNotificationIsChecked.set -> void
+Dynamo.Wpf.UI.CustomSaveFileDialog.FileOk -> event System.ComponentModel.CancelEventHandler
+Dynamo.Wpf.UI.CustomSaveFileDialog.OverwritePrompt.get -> bool
+Dynamo.Wpf.UI.CustomSaveFileDialog.OverwritePrompt.set -> void
 Dynamo.Wpf.UI.ToastManager
 Dynamo.Wpf.UI.ToastManager.CloseRealTimeInfoWindow() -> void
 Dynamo.Wpf.UI.ToastManager.CreateRealTimeInfoWindow(string content, bool stayOpen = false, string headerText = "", string hyperlinkText = "", System.Uri hyperlinkUri = null, System.Uri fileLinkUri = null) -> void