Skip to content

WpfGfx: Mitigate RemoveChild crash #11465

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
etvorun wants to merge 1 commit into
base: main
Choose a base branch
from
Open

WpfGfx: Mitigate RemoveChild crash #11465

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/Microsoft.DotNet.Wpf/src/WpfGfx/core/resources/node.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,14 @@ CMilVisual::RemoveChild(

if (!m_rgpChildren.Remove(pChild))
{
// We've seen a handful of Watson dumps where incoming child is already detached
// from parent and about to be deleted by the next MilCmdChannelDeleteResource
// command. There are no known repros. For now we'll pretend we processed it.
Comment on lines +323 to +325
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment references "Watson dumps" which is Microsoft's internal crash reporting system. Consider whether this terminology should be more generic for the open-source version of WPF, or if it should reference the specific issue number (Issue #11464) instead for better traceability.

Suggested change
// We've seen a handful of Watson dumps where incoming child is already detached
// from parent and about to be deleted by the next MilCmdChannelDeleteResource
// command. There are no known repros. For now we'll pretend we processed it.
// We've seen a handful of crash reports (see Issue #11464) where the incoming
// child is already detached from its parent and about to be deleted by the next
// MilCmdChannelDeleteResource command. There are no known repros. For now we'll
// pretend we processed it.

Copilot uses AI. Check for mistakes.
if (pChild->GetParent() == NULL)
{
return S_OK;
}

Comment on lines +325 to +330
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic here is incomplete and could cause silent failures. When m_rgpChildren.Remove(pChild) fails, there are three possible states:

  1. pChild->GetParent() == NULL (already detached) - the code handles this
  2. pChild->GetParent() == this (inconsistent state: child thinks it has this parent, but isn't in the collection) - this falls through to E_INVALIDARG correctly
  3. pChild->GetParent() == someOtherParent (child belongs to a different parent) - this currently returns S_OK inappropriately

The check should verify that pChild->GetParent() is either NULL (already detached) or equal to this (inconsistent state to fail on). If the parent is a different visual, this is a programming error and should fail with E_INVALIDARG.

Suggested change
// command. There are no known repros. For now we'll pretend we processed it.
if (pChild->GetParent() == NULL)
{
return S_OK;
}
// command. There are no known repros. For now we'll pretend we processed it
// only if the child no longer has a parent. Any other parent state is a
// programming error and should fail.
CMilVisual *pParent = pChild->GetParent();
if (pParent == NULL)
{
return S_OK;
}
// If the child still has a parent at this point, then either:
// - pParent == this: the child thinks it belongs to this visual but is
// missing from m_rgpChildren, or
// - pParent != this: the child belongs to a different visual.
// Both represent inconsistent or invalid state and are treated as errors.
if (pParent != this)
{
IFC(E_INVALIDARG);
}

Copilot uses AI. Check for mistakes.
IFC(E_INVALIDARG);
}

Expand Down