DYN-9707: Harden graph locking against corrupt lock files and improve Save As safety by ivaylo-matov · Pull Request #17197 · DynamoDS/Dynamo

ivaylo-matov · 2026-06-29T12:39:26Z

Purpose

This PR is related to DYN-9707 and addresses this post plus a comment from a recent catchup. The changes improve the behaviour of the graph locking mechanism that prevents two Dynamo instances from opening and editing the same .dyn or .dyf file at the same time.

When Dynamo opens a graph file it writes a small sidecar file (.dynlock) to signal that the file is in use. If another Dynamo instance detects that sidecar, it shows a prompt asking the user to either cancel or save a copy of the graph to work on instead. Several edge cases in that flow were not handled correctly and are addressed here.

Improvements to previous logic:

Corrupt lock files no longer block the user:
A .dynlock file that cannot be read (e.g. left behind by a crash or a disk error) was previously treated the same as a live lock, triggering the conflict prompt with no useful owner information. It is now treated as abandoned and silently overwritten, so the graph opens normally.
Transient read failures no longer drop a valid lock:
If an antivirus scan or a network hiccup briefly prevents Dynamo from reading its own .dynlock during a heartbeat cycle, the lock is now retried rather than immediately abandoned. The lock is only released after three consecutive failures — at which point the file would already appear stale to other instances anyway.
Lock freshness check is faster:
The heartbeat interval was halved from 30 seconds to 15 seconds, reducing the time another instance has to wait before it can safely take over a lock left behind by a crashed Dynamo (from 90 seconds down to 45 seconds).
Save As dialog is safer:
When the user chooses to save a copy, the dialog now prevents them from accidentally picking the same file that is already locked. It also restores the standard "this file already exists, do you want to replace it?" warning for any other existing file they choose, which was inadvertently suppressed.
All user-facing strings are now localizable:
The messages shown in the conflict dialogs have been moved from hardcoded English text into resource files.
Regression tests cover the new behaviours: corrupt lock handling, heartbeat resilience, and the save-as dialog validation.

Declarations

Check these if you believe they are true

Is documented according to the standards
The level of testing this PR includes is appropriate
Changes to the API follow Semantic Versioning and are documented in the API Changes document.

Release Notes

Fixed an issue where a corrupted graph lock file could incorrectly block a user from opening a graph.

Reviewers

@DynamoDS/eidos
@jasonstratton
@johnpierson

FYIs

@dnenov

github-actions

See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-9707

Copilot

Pull request overview

This PR strengthens Dynamo’s graph-lock sidecar (.dynlock) behavior in DynamoCore and refines the WPF conflict “Save As” UX in DynamoCoreWpf, with accompanying regression tests and localized strings.

Changes:

Treat unreadable/corrupt .dynlock files as reclaimable and add heartbeat resilience (shorter interval + consecutive failure handling).
Harden the WPF “Save As” flow to prevent selecting the locked source file and restore overwrite confirmations.
Add new unit tests covering corrupt lock handling, heartbeat mismatch behavior, and Save As validation; move new UI strings into resources.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
test/DynamoCoreWpfTests/WpfGraphLockUserPromptTests.cs	Adds WPF tests for Save As validation and overwrite confirmation behavior.
test/DynamoCoreTests/Graph/Workspaces/GraphLockManagerTests.cs	Adds tests for corrupt lock takeover and heartbeat/session-mismatch behavior.
src/DynamoCoreWpf/UI/CustomSaveFileDialog.cs	Extends `IFileSaver` / `CustomSaveFileDialog` with `FileOk` + `OverwritePrompt` for testability and validation hooks.
src/DynamoCoreWpf/Services/WpfGraphLockUserPrompt.cs	Implements Save As path blocking for the locked source file and custom overwrite confirmation.
src/DynamoCoreWpf/PublicAPI.Unshipped.txt	Declares newly exposed `CustomSaveFileDialog` members in the public API tracking file.
src/DynamoCoreWpf/Properties/Resources.resx	Adds localized strings for Save As validation and overwrite confirmation.
src/DynamoCoreWpf/Properties/Resources.en-US.resx	Adds en-US localized strings for Save As validation and overwrite confirmation.
src/DynamoCoreWpf/Properties/Resources.Designer.cs	Updates resource designer accessors for the new strings.
src/DynamoCore/Graph/Workspaces/Locking/GraphLockManager.cs	Updates lock acquisition/heartbeat logic (interval, reclaim policy, retry/drop behavior).

Files not reviewed (1)

src/DynamoCoreWpf/Properties/Resources.Designer.cs: Generated file

ivaylo-matov · 2026-06-29T17:25:47Z

            if (!readable)
            {
-                return null;
+                // The lock file exists but could not be parsed after all retry attempts.
+                // This indicates a corrupt file rather than a live owner.
+                // Treat it the same as a stale lock: overwrite and take ownership.
+                GraphLockFile.WriteHeartbeat(sidecarPath, info);
+                RegisterOwnedLock(normalizedPath, sidecarPath, info, workspace);
+                return GraphLockAcquireResult.Acquired(normalizedPath);
            }


Fair point. This is now addressed with GraphLockReadResult

I think the fix you applied is correct, but I think there is a missing case that I flagged in GraphLockManager.CreateAndOpenCopy() that you had applied in GraphLockManager.TryAcquireReadableLock() for the TransientFailure case.

ivaylo-matov · 2026-06-29T17:26:26Z

+                // OverwritePrompt is false so we must replicate the OS overwrite warning
+                // for any other existing file the user may have chosen.
+                if (File.Exists(chosenPath))
+                {
+                    var confirm = showMessageBox(
+                        Path.GetFileName(chosenPath) + Resources.ConfirmReplaceFileMessage,
+                        Resources.ConfirmReplaceFileTitle,
+                        MessageBoxButtons.YesNo,
+                        MessageBoxIcon.Warning);


... now addressed properly.
Looks updated here.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

…ilename-is-modified

sonarqubecloud · 2026-06-29T18:53:00Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

jasonstratton

Honestly this is a pretty complicated solution to a gnarly problem. SO it is great work, but I did find a couple of things that should be addressed, I think. ... Please feel free to push back if you don't agree. As I said, it is pretty complicated and you understand it much more intimately than I. Thanks.

jasonstratton · 2026-07-01T03:26:31Z

+                return GraphLockAcquireResult.Acquired(normalizedPath);
+            }
+
+            if (readResult == GraphLockReadResult.TransientFailure)


This fix addressing the TransierntFailure should probably be applied below as well, look at line 322

jasonstratton · 2026-07-01T03:27:33Z

                        ownsSaveAsLock = true;
                    }
-                    else if (!readable || IsStale(saveAsLock) || IsDeadLocalProcess(saveAsLock))
+                    else if (saveAsReadResult == GraphLockReadResult.Corrupt || IsStale(saveAsLock) || IsDeadLocalProcess(saveAsLock))


So here, you might want to check for the TransientFailure as well?
Or maybe guard against it before this if statement?

jasonstratton · 2026-07-01T03:33:16Z

            if (!readable)
            {
-                return null;
+                // The lock file exists but could not be parsed after all retry attempts.
+                // This indicates a corrupt file rather than a live owner.
+                // Treat it the same as a stale lock: overwrite and take ownership.
+                GraphLockFile.WriteHeartbeat(sidecarPath, info);
+                RegisterOwnedLock(normalizedPath, sidecarPath, info, workspace);
+                return GraphLockAcquireResult.Acquired(normalizedPath);
            }


I think the fix you applied is correct, but I think there is a missing case that I flagged in GraphLockManager.CreateAndOpenCopy() that you had applied in GraphLockManager.TryAcquireReadableLock() for the TransientFailure case.

jasonstratton · 2026-07-01T03:34:52Z

                        GraphLockFile.WriteHeartbeat(sidecarPath, info);
                        ownsSaveAsLock = true;
                    }
                    else


So if TransientFailure is not handled above, it will be handled by this else clause, which cancels the lock and it gets stolen.

jasonstratton · 2026-07-01T03:39:57Z

                    GraphLockInfo current;
-                    if (!GraphLockFile.TryRead(owned.SidecarPath, out current) ||
-                        current.SessionId != owned.Info.SessionId)
+                    if (GraphLockFile.TryRead(owned.SidecarPath, out current) != GraphLockReadResult.Ok)


If TryRead() returns NotFound ... it's pretty definitive that it does not exist and the retries are unnecessary. It's probably best to break out of the retries right away as it might allow another session to claim the path.

Similarly returning Corrupt would also retry, but the file would not suddenly uncorrupt itself ... Elsewhere in AcquireLockInternal() treats Corrupt as something that can be immediately reclaimed. This might be a window where two sessions claim the same path.

ivaylo-matov · 2026-07-03T04:43:26Z

Closing this PR as agreed. I'll revert the changes from the other "graph lock" PRs and resubmit them after the 4.2 release.

ivaylo-matov added 4 commits June 29, 2026 08:36

save as behavior - register public APIs

1512227

graphLockManager improvements

d039d15

resources + ensure overwrite dialog is called for all existing dyn

aa2db99

regression tests

0d1d43d

github-actions Bot reviewed Jun 29, 2026

View reviewed changes

jasonstratton requested a review from Copilot June 29, 2026 13:38

Copilot started reviewing on behalf of jasonstratton June 29, 2026 13:39 View session

Copilot AI reviewed Jun 29, 2026

View reviewed changes

ivaylo-matov and others added 3 commits June 29, 2026 16:20

malformed xml summary

024074c

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

comments addressed

af1abec

Merge branch 'master' into DYN-9707-Disable-SaveAs-button-utill-the-f…

a880bd1

…ilename-is-modified

jasonstratton requested changes Jul 1, 2026

View reviewed changes

ivaylo-matov closed this Jul 3, 2026

Uh oh!

Conversation

ivaylo-matov commented Jun 29, 2026

Purpose

Declarations

Release Notes

Reviewers

FYIs

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

ivaylo-matov Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jasonstratton Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ivaylo-matov Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Jun 29, 2026

Quality Gate passed

Uh oh!

jasonstratton left a comment

Choose a reason for hiding this comment

Uh oh!

jasonstratton Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

jasonstratton Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

jasonstratton Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

jasonstratton Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

jasonstratton Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

jasonstratton Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

ivaylo-matov commented Jul 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ivaylo-matov Jun 29, 2026 •

edited

Loading

ivaylo-matov Jun 29, 2026 •

edited

Loading