dotnet / dotnet/wpf

[WpfGfx] RemoveChild should tolerate already-detached children

Open
#11,464 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C#
Stars
7.7k
Forks
1.3k
Avg merge
1d 11h
Merged PRs (30d)
61

Description

# [WpfGfx] RemoveChild should tolerate already-detached children

## Summary
A crash can occur when visual tree removal processes a child that has already been detached from its parent.

## Observed behavior
- In rare timing/order scenarios, `CMilVisual::RemoveChild` receives a child that is no longer present in the parent collection.
- The current path treats this as invalid and fails, which can terminate the process.

## Repro notes
1. Use a rendering-heavy visual tree scenario with frequent attach/detach operations.
2. Trigger composition command processing where a remove command can arrive after the child has already been detached.
3. Observe failure in the `RemoveChild` path when the child is not found in the parent list.

## Impact
- Improves reliability in visual tree mutation paths under composition pressure.
- Reduces crash risk for apps with dynamic visuals and frequent child removal.

## Expected behavior
Removing an already-detached child should be treated as a safe no-op.

## Actual behavior
The code reports invalid argument and fails when the child is absent from the parent collection.

## Suspected root cause
A remove request can target a child that has already been detached, but the method currently treats this state as a hard error rather than an idempotent operation.

## Proposed fix direction
- In `CMilVisual::RemoveChild`, when removal from `m_rgpChildren` fails, check whether `pChild->GetParent()` is `NULL`.
- If already detached, return success and treat the operation as no-op.
- Keep existing invalid-argument behavior for genuinely inconsistent states where the child still reports a parent.

Contributor guide

Open the contributing guide

Research direction

Start at CMilVisual::RemoveChild and inspect the m_rgpChildren removal path and pChild->GetParent() state. Reproduce or run the existing visual-tree/composition tests if available; done means an already-detached child is a no-op while a child that still reports a parent keeps the invalid-argument failure.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
desktop
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.