Fix context menu disposal and reorganize CLSCompliant attribute - #334
Merged
Conversation
Defer disposal of the root ContextMenuWithHandler in MenuItemWithHandler.OnClick via BeginInvoke so that Windows Forms can finish its own closing sequence (SetVisibleCore / Handle access) before the object is released. The immediate Dispose() call raced with the WinForms teardown path and caused ObjectDisposedException. The IsDisposed guard in the lambda also handles the case where the Closed-event handler (constructor) disposes first, so the deferred call becomes a no-op rather than a double-dispose. Co-Authored-By: Claude <noreply@anthropic.com>
The declaration was living inside the vendored PowerCollections source (Algorithms.cs), which is fragile — it was only there because the file was originally a standalone library. Moving it to Properties/AssemblyInfo.cs makes the intent explicit and project-controlled. All seven existing [CLSCompliant(false)] annotations are kept because they are each genuinely required: - Gl class: class-level opt-out for hundreds of P/Invoke methods with sbyte/ushort/uint parameters - Glu.gluNurbsCurve, DebuggerContainer.Show: float[,] / GeoPoint[,] multidimensional array parameters (excluded from CLS) - Gdi.ABC struct: contains a public uint field - Gdi.GetCharABCWidths, GetCharWidth32: uint parameters - Gdi._SetPixelFormat: underscore-prefixed public identifier (CLS Rule 3) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves WinForms context menu lifecycle handling to avoid disposal-related UI issues, and standardizes assembly-level metadata placement by relocating the CLS compliance attribute into a dedicated AssemblyInfo file.
Changes:
- Deferred disposal of the root
ContextMenuWithHandlerfromMenuItemWithHandler.OnClick()viaBeginInvoke()to let WinForms complete menu-closing processing. - Added defensive checks before disposing the root context menu (
null/disposed guarding). - Moved
[assembly: CLSCompliant(true)]out ofAlgorithms.csintoCADability/Properties/AssemblyInfo.cs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| CADability/Properties/AssemblyInfo.cs | Adds dedicated assembly-level CLS compliance attribute location. |
| CADability/Algorithms.cs | Removes in-file assembly attribute now handled by AssemblyInfo. |
| CADability.Forms/MenuManager.cs | Defers root context menu disposal to reduce close-sequence disposal hazards. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR addresses two issues: a potential race condition in context menu disposal and improves code organization by moving the CLSCompliant assembly attribute to a dedicated AssemblyInfo.cs file.
Key Changes
Deferred context menu disposal: Modified
MenuManager.OnClick()to defer the disposal of the rootContextMenuWithHandlerusingBeginInvoke(). This ensures WinForms can complete its menu closing sequence before the object is released, preventing potential issues with blank menu text accumulation.Added null/disposed checks: Added explicit checks to verify the root menu exists and hasn't already been disposed before attempting disposal, making the code more defensive.
Reorganized CLSCompliant attribute: Moved the
[assembly: CLSCompliant(true)]attribute fromAlgorithms.csto a new dedicatedProperties/AssemblyInfo.csfile, following standard .NET conventions for assembly-level attributes.Implementation Details
The context menu disposal now uses
BeginInvoke()to schedule disposal on the UI thread after the current message is processed, allowing WinForms to safely complete its internal cleanup before the menu object is released. This prevents potential ObjectDisposedException or state corruption issues that could occur from immediate disposal during the menu closing sequence.https://claude.ai/code/session_01JR5bGFDY4UYtSUEotBcu8z