DYN-10409 Fix SynchronizationLockException crash on Save As in C3D#17071
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-10409
There was a problem hiding this comment.
Pull request overview
Fixes a WPF threading crash (DYN-10409) by ensuring DelegateCommand.RaiseCanExecuteChanged raises CanExecuteChanged on the UI dispatcher, and adds regression coverage plus agent-skill documentation updates.
Changes:
- Marshal
CanExecuteChangedto the captured WPF dispatcher when raised from background threads. - Add
DelegateCommandthreading regression tests covering background-thread, UI-thread, and concurrent raises. - Add two new agent skills (
dynamo-build-tests,dynamo-run-tests) and reference them from agent docs.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/DynamoCoreWpf/Utilities/DelegateCommand.cs |
Dispatches CanExecuteChanged onto the dispatcher to avoid cross-thread WPF CommandManager crashes. |
test/DynamoCoreWpfTests/DelegateCommandTests.cs |
Adds regression tests validating thread marshalling and concurrency behavior for RaiseCanExecuteChanged. |
AGENTS.md |
Documents the newly added skills in the repo-level agent guidance. |
.agents/skills/dynamo-run-tests/SKILL.md |
Adds a canonical workflow for running NUnit tests via dotnet test with repo-specific constraints. |
.agents/skills/dynamo-build-tests/SKILL.md |
Adds a canonical workflow for building test projects reliably (MSBuild-first) in this repo. |
.agents/README.md |
Updates the cross-tool parity matrix and skill list to include the new skills. |
DelegateCommand.RaiseCanExecuteChanged fired CanExecuteChanged directly on whatever thread called it. When invoked from the Dynamo scheduler thread (e.g. after graph execution triggered by a Save As workspace reload), WPF's CommandManager.FindCommandBinding attempted to exit a Monitor it never entered, crashing with SynchronizationLockException. Marshal the event to the UI thread via Dispatcher.BeginInvoke when called from a non-UI thread. UI-thread callers continue to invoke synchronously. Adds regression tests in DelegateCommandTests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tion.Current Application.Current is null in DynamoTestUIBase tests (no WPF Application is created), causing RaiseCanExecuteChanged to fall through to the direct-invoke branch and fire on the background thread. Capturing Dispatcher.CurrentDispatcher at DelegateCommand construction time is more robust: commands are always created on the UI thread, so the captured dispatcher is always correct regardless of whether Application.Current exists. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Re-read CanExecuteChanged inside BeginInvoke lambda rather than capturing a snapshot before posting, so unsubscriptions that happen in the window between scheduling and execution are respected. - Pass explicit timeoutSeconds: 5 to DoEventsLoop in both regression tests so failures surface in seconds rather than stalling the suite for the 60-second default timeout. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jasonstratton
force-pushed
the
DYN-10409-C3D-Save-File-Crash
branch
from
12a1083 to
d9d1ff6
Compare
|
RobertGlobant20
left a comment
RobertGlobant20
left a comment
There was a problem hiding this comment.
LGTM, your comment about "The crash is a race condition and is not deterministically reproducible", then is possible to test it? or were you able to reproduce the crash and test your fix?
|
I was unable to test it myself. That's why the DynaNotes indicates "Suggested" steps to reproduce. 😆 |
|
ok then you just need to update your branch to see if Dynamo Selfserve pass |
|
Whoops! Thought I had rerun SelfServe previously, but did not. Rerunning it now: |
Is it better to update the branch or to rerun the failing job? |
|
Dynamo Selfserve passed Merging now |
|
/cherrypck |
|
/cherrypick |
|
Successfully created backport PR for |
Purpose
Fixes DYN-10409: Dynamo for Civil 3D crashes with an unhandled
SynchronizationLockExceptioninside WPF'sCommandManager.FindCommandBindingafter performing a Save As to a new.dynfile.Root cause:
DelegateCommand.RaiseCanExecuteChangedfiredCanExecuteChangeddirectly on whatever thread called it. When the Dynamo scheduler thread completes graph execution (triggered bySetPeriodicEvaluationafter the Save As workspace reload), WPF'sCommandManagerassumesCanExecuteChangedis always raised on the UI thread — raising it from a background thread corrupts the internalMonitorlock state, causingMonitor.Exitto throw withSynchronizationLockException.Fix: Capture
Dispatcher.CurrentDispatcheratDelegateCommandconstruction time (commands are always constructed on the UI thread).RaiseCanExecuteChangednow checks_dispatcher.CheckAccess()and marshals viaBeginInvokewhen called from a non-UI thread. UI-thread callers continue to invoke synchronously.Declarations
Check these if you believe they are true
Release Notes
Fixed a crash (
SynchronizationLockExceptionin WPF'sCommandManager) that occurred in Dynamo for Civil 3D after performing a Save As to a new file while running in Automatic evaluation mode.Reviewers
@DynamoDS/eidos
Additional notes: The bug has been latent since
DelegateCommandwas introduced in January 2015 but surfaces reliably under conditions now common in C3D: Automatic run mode, Save As to a cloud-synced path (OneDrive), and thesearchbar_separate_threadfeature flag enabled. Regression tests added inDelegateCommandTests.csusingDynamoTestUIBase+DispatcherUtil.DoEventsLoop.FYIs
Notes can be found here: https://git.autodesk.com/strattj/DynaNotes/blob/main/DYN-10409/README.md