9348: DYN-9742 PackagePath Crash Fix #16642
Conversation
Due that Revit when launching Dynamo the first time creates the PathManager with parameters then the second time launching Dynamo is throwing an exception saying that was already initialized, then in this fix I'm updating the code to not throwing the exception and do nothing if was already initialized.
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-9348
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a crash in Revit that occurs when Dynamo is opened, closed, and reopened. The root cause was the PathManager throwing an InvalidOperationException when attempting to reinitialize, which was blocking Revit's ability to launch Dynamo multiple times in the same session.
Key Changes:
- Modified the
PathManager.Initialize()method to allow silent reinitialization instead of throwing an exception - Changed the initialization check from throwing an error to simply doing nothing when already initialized
| lazy = new Lazy<PathManager>(() => new PathManager(parameters)); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The change from throwing an exception to silently ignoring reinitialization is a significant behavioral change that could mask configuration errors. If Revit attempts to reinitialize PathManager with different parameters on the second launch, those parameters will be silently ignored, potentially leading to subtle bugs. Consider logging a warning when reinitialization is attempted, or validating that the new parameters match the existing instance's parameters before ignoring the call.
There was a problem hiding this comment.
I agree with this comment but due that we are breaking the lazy loading Singleton pattern by leaving the PathManager constructor as internal (so we can create several instances) and also using the static Initialize() method for passing parameters to constructor, this comment doesn't apply.
There was a problem hiding this comment.
Yes @RobertGlobant20. I’d like to add that it’s not currently possible (nor part of Revit’s scenario) for Revit to pass different parameters on the second launch, so this approach is fine and completely safe. In the future, we should make PackageManager a concrete instance within Dynamo, with a defined and predictable initialization sequence (and ensure that PackageManager is disposed of along with the Dynamo instance).
Looking at these changes, could you please help test this in the Civil 3D scenario? The code appears to be creating a new PathManager object before the DynamoModel constructor is invoked. With the latest code, it shouldn’t throw an exception anymore, but would PathManager still honor the values passed through the DynamoModel.CreatePathManager method?
Thank you! 🙏
There was a problem hiding this comment.
@benglin in the first paragraph you were trying to say "PathManager" instead of "PackageManager", right?
About testing in Civil3D I think is not possible yet, until Civil3D is integrated with Dynamo 4.0, then we will need some weeks more.
|
This is a GIF showing the expected behavior in Revit Preview Release and Dynamo 4.1.0 |
|
Waiting to merge once SelfServe pass |
(cherry picked from commit c73536f)
|
Successfully created backport PR for |
Purpose
Fixing Revit crash when opening Dynamo close it and re-open Dynamo again.
Due that Revit when launching Dynamo the first time creates the PathManager with parameters then the second time launching Dynamo is throwing an exception saying that was already initialized, then in this fix I'm updating the code to not throwing the exception and do nothing if was already initialized.
Declarations
Check these if you believe they are true
Release Notes
Fixing Revit crash when opening Dynamo close it and re-open Dynamo again.
Reviewers
@QilongTang @benglin @zeusongit
FYIs