Skip to content

Commit cf37f69

Browse files
Ahamed-AliPureWeen
authored andcommitted
Fixed the RealParent Warning shown issue (#30156)
* Safe Get of RealParent to avoid warnings * Optimized the fix * Make it as default private * Optimized the fix * Modified the fix and added test * Modified the fix based on concern and added test * unwanted spacing * Replaced the Parent property directly
1 parent ee49d49 commit cf37f69

File tree

2 files changed

+149
-19
lines changed

2 files changed

+149
-19
lines changed

src/Controls/src/Core/Element/Element.cs

Lines changed: 28 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -333,30 +333,39 @@ internal Element ParentOverride
333333
}
334334

335335
WeakReference<Element> _realParent;
336-
/// <summary>For internal use by .NET MAUI.</summary>
337-
[EditorBrowsable(EditorBrowsableState.Never)]
338-
public Element RealParent
336+
Element TryGetRealParent(bool logWarningIfParentHasBeenCollected = true)
339337
{
340-
get
338+
var realParent = _realParent;
339+
if (realParent is null)
341340
{
342-
if (_realParent is null)
343-
{
344-
return null;
345-
}
346-
if (_realParent.TryGetTarget(out var parent))
347-
{
348-
return parent;
349-
}
350-
else
341+
return null;
342+
}
343+
if (realParent.TryGetTarget(out var parent))
344+
{
345+
return parent;
346+
}
347+
else
348+
{
349+
// Clear the weak reference since the target has been garbage collected
350+
// This prevents repeated checks and warnings on subsequent accesses
351+
_realParent = null;
352+
if (logWarningIfParentHasBeenCollected)
351353
{
352354
Application.Current?
353-
.FindMauiContext()?
354-
.CreateLogger<Element>()?
355-
.LogWarning($"The RealParent on {this} has been Garbage Collected. This should never happen. Please log a bug: https://github.com/dotnet/maui");
355+
.FindMauiContext()?
356+
.CreateLogger<Element>()?
357+
.LogWarning($"The RealParent on {this} has been Garbage Collected. This should never happen. Please log a bug: https://github.com/dotnet/maui");
356358
}
357-
358-
return null;
359359
}
360+
361+
return null;
362+
}
363+
364+
/// <summary>For internal use by .NET MAUI.</summary>
365+
[EditorBrowsable(EditorBrowsableState.Never)]
366+
public Element RealParent
367+
{
368+
get => TryGetRealParent();
360369
private set
361370
{
362371
if (value is null)
@@ -386,7 +395,7 @@ public Element Parent
386395

387396
void SetParent(Element value)
388397
{
389-
Element realParent = RealParent;
398+
Element realParent = TryGetRealParent(false);
390399

391400
if (realParent == value)
392401
{

src/Controls/tests/Core.UnitTests/ElementTests.cs

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,5 +313,126 @@ public void ChildRemoved()
313313
root.Children.Remove(child);
314314
Assert.True(removed);
315315
}
316+
317+
[Fact]
318+
public void RealParent_ReturnsNullAfterParentGarbageCollected()
319+
{
320+
var child = new TestElement();
321+
WeakReference parentRef;
322+
323+
// Create parent in separate scope to enable GC
324+
void CreateParent()
325+
{
326+
var parent = new TestElement();
327+
parentRef = new WeakReference(parent);
328+
parent.Children.Add(child);
329+
// parent goes out of scope here
330+
}
331+
332+
CreateParent();
333+
334+
// Force garbage collection
335+
GC.Collect();
336+
GC.WaitForPendingFinalizers();
337+
GC.Collect();
338+
339+
// Verify parent was collected
340+
Assert.False(parentRef.IsAlive);
341+
342+
// RealParent should return null and not throw
343+
Assert.Null(child.RealParent);
344+
}
345+
346+
[Fact]
347+
public void SetParent_DoesNotLogWarningWhenParentGarbageCollected()
348+
{
349+
var child = new TestElement();
350+
WeakReference parentRef;
351+
352+
// Create parent in separate scope
353+
void CreateParent()
354+
{
355+
var parent = new TestElement();
356+
parentRef = new WeakReference(parent);
357+
parent.Children.Add(child);
358+
}
359+
360+
CreateParent();
361+
362+
// Force garbage collection
363+
GC.Collect();
364+
GC.WaitForPendingFinalizers();
365+
GC.Collect();
366+
367+
// Setting parent should not log warnings internally
368+
var newParent = new TestElement();
369+
newParent.Children.Add(child);
370+
371+
Assert.Same(newParent, child.RealParent);
372+
}
373+
374+
[Fact]
375+
public void RealParent_ReturnsNull_WhenNoParentSet()
376+
{
377+
var element = new TestElement();
378+
Assert.Null(element.RealParent);
379+
}
380+
381+
[Fact]
382+
public void RealParent_IsThreadSafe_WhenAccessedConcurrently()
383+
{
384+
var element = new TestElement();
385+
var parent = new TestElement();
386+
parent.Children.Add(element);
387+
388+
// Act - Create multiple threads accessing and modifying RealParent concurrently
389+
const int ThreadCount = 10;
390+
var threads = new System.Threading.Thread[ThreadCount];
391+
var exceptions = new List<Exception>();
392+
var threadBarrier = new System.Threading.Barrier(ThreadCount);
393+
394+
for (int i = 0; i < ThreadCount; i++)
395+
{
396+
threads[i] = new System.Threading.Thread(() =>
397+
{
398+
try
399+
{
400+
// Synchronize threads to increase chance of concurrent access
401+
threadBarrier.SignalAndWait();
402+
403+
// Some threads read the parent
404+
element.RealParent?.ToString();
405+
406+
// Clear reference to parent so GC can collect it
407+
parent = null;
408+
409+
// Force garbage collection on some threads
410+
GC.Collect();
411+
GC.WaitForPendingFinalizers();
412+
GC.Collect();
413+
414+
// Try to access the parent again after potential collection
415+
element.RealParent?.ToString();
416+
}
417+
catch (Exception ex)
418+
{
419+
lock (exceptions)
420+
{
421+
exceptions.Add(ex);
422+
}
423+
}
424+
});
425+
threads[i].Start();
426+
}
427+
428+
// Wait for all threads to complete
429+
foreach (var thread in threads)
430+
{
431+
thread.Join();
432+
}
433+
434+
// Assert - No exceptions should be thrown during concurrent access
435+
Assert.Empty(exceptions);
436+
}
316437
}
317438
}

0 commit comments

Comments
 (0)