[Controls] Fix BindableObject DefaultValueCreator re-entrancy regression

[PureWeen]

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Summary

Fixes a regression introduced by #33584 in which a BindableProperty whose value comes from a DefaultValueCreator could permanently return its raw DefaultValue (and report IsSet == false) when that creator re-entrantly sets other properties.

This is the main counterpart to the servicing-branch revert #35970. On release/10.0.1xx-sr8 the safest action was to fully revert #33584. On main we instead fix the bug directly, because #33584 also introduced internal APIs that other code now depends on (see below) — a straight revert would remove them.

Root cause

#33584 rewrote GetOrCreateContext (non-NETSTANDARD path) to use CollectionsMarshal.GetValueRefOrAddDefault:

ref var context = ref CollectionsMarshal.GetValueRefOrAddDefault(_properties, property.InternalId, out var exists);
if (!exists)
    context = CreateContext(property);   // <-- runs the DefaultValueCreator

GetValueRefOrAddDefault returns a ref into the dictionary's backing array. CreateContext then invokes the property's DefaultValueCreator, which can re-entrantly SetValue other properties. Those additions can grow _properties past its capacity, reallocating the backing array. The held ref now points at the old, discarded array, so context = CreateContext(...) writes there and the live dictionary slot for the property stays null.

Net effect:

• 1st GetValue returns the creator's value (the freshly created context is returned directly).
• 2nd GetValue finds the null slot and falls back to property.DefaultValue.
• IsSet returns false.

The fix

Create the context before inserting it, holding no ref across CreateContext — exactly the shape the #if NETSTANDARD branch already used:

var context = GetContext(property);
if (context is null)
{
    context = CreateContext(property);
    _properties.Add(property.InternalId, context);
}
return context;

The two TFM branches are unified and the now-unused using System.Runtime.InteropServices; is removed. The hot path (GetContext, already-exists case) is unchanged.

Why fix instead of revert on main

Reverting #33584 here would remove internal API that is in use:

• BindableProperty.InternalId and the Dictionary<int, BindablePropertyContext> keying model.
• BindableObject.GetValues<T>(BindableProperty[]), consumed internally by ShellAppearance (and useful to tooling).

This PR preserves all of those — only GetOrCreateContext changes.

Tests

• DefaultValueCreatorCachesValueWhenReentrantPropertyAddsResizeStore — regression test that forces a dictionary resize during default-value creation and asserts both reads return the created value, the creator runs once, and IsSet is true.
• GetValuesReturnsSetStateAndValue — first direct unit coverage for the internal GetValues<T> API (the existing GetValues test never actually called it).

The LocalValueEnumerator-based test from #35970 is intentionally omitted here — that API only exists on the reverted servicing branch, not on main.

Verified locally (net10.0)

Code	Regression test
main as-is (#33584 optimization)	❌ FAIL — Assert.Equal() Expected: 42, Actual: 0
main + this fix	✅ PASS (both new tests)

Related

• BindableObject property access micro-optimizations #33584 — original micro-optimization that introduced the regression.
• Revert PR #33584: restore BindableObject property-context lookup model #35970 — servicing-branch revert on release/10.0.1xx-sr8.

[github-actions]

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36030

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36030"

[MauiBot]

AI Review Summary

@PureWeen — new AI review results are available based on this last commit: ee79130. To request a fresh review after new comments or commits, comment /review rerun.

Review Sessions — click to expand

Gate — Test Before & After Fix

Gate Result: ✅ PASSED

Platform: ANDROID · Base: main · Merge base: b4d4b25b

Test	Without Fix (expect FAIL)	With Fix (expect PASS)
🧪 BindableObjectUnitTests BindableObjectUnitTests	✅ FAIL — 158s	✅ PASS — 108s

🔴 Without fix — 🧪 BindableObjectUnitTests: FAIL ✅ · 158s

  Determining projects to restore...
  Restored /home/vsts/work/1/s/src/TestUtils/src/TestUtils/TestUtils.csproj (in 4.61 sec).
  Restored /home/vsts/work/1/s/src/Graphics/src/Graphics/Graphics.csproj (in 134 ms).
  Restored /home/vsts/work/1/s/src/Essentials/src/Essentials.csproj (in 4.68 sec).
  Restored /home/vsts/work/1/s/src/Core/maps/src/Maps.csproj (in 11.24 sec).
  Restored /home/vsts/work/1/s/src/Core/src/Core.csproj (in 295 ms).
  Restored /home/vsts/work/1/s/src/Controls/src/Xaml/Controls.Xaml.csproj (in 72 ms).
  Restored /home/vsts/work/1/s/src/Controls/src/Core/Controls.Core.csproj (in 47 ms).
  Restored /home/vsts/work/1/s/src/Controls/Maps/src/Controls.Maps.csproj (in 46 ms).
  Restored /home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj (in 2.26 sec).
  1 of 10 projects are up-to-date for restore.
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14432623
  Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14432623
  Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14432623
  Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
  Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14432623
  Maps -> /home/vsts/work/1/s/artifacts/bin/Maps/Debug/net10.0/Microsoft.Maui.Maps.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14432623
  Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14432623
  Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0/Microsoft.Maui.Controls.Xaml.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14432623
  Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0/Microsoft.Maui.Controls.Maps.dll
  TestUtils -> /home/vsts/work/1/s/artifacts/bin/TestUtils/Debug/netstandard2.0/Microsoft.Maui.TestUtils.dll
  Controls.Core.UnitTests -> /home/vsts/work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll
Test run for /home/vsts/work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (x64)

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 10.0.0)
[xUnit.net 00:00:00.27]   Discovering: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:02.50]   Discovered:  Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:02.52]   Starting:    Microsoft.Maui.Controls.Core.UnitTests
  Passed GetValuesDefaults [24 ms]
  Passed EventSubscribingOnBindingContextChanged [25 ms]
  Passed RecursiveChange [2 ms]
  Passed IsSetIsTrueWhenPropSet [< 1 ms]
  Passed DefaultValueCreator [3 ms]
  Passed SetValueCoreInvokesOpImplicitOnPropertyType [1 ms]
  Passed StyleBindingIsOverridenByValue [6 ms]
  Passed BindingsEditableAfterUnapplied [1 ms]
  Passed SetValueInvalid [< 1 ms]
  Passed StyleBindingIsOverridenByStyleBinding [< 1 ms]
  Passed RaiseOnEqual [1 ms]
  Passed SetValueCoreInvokesOpImplicitOnValue [< 1 ms]
  Passed StyleDynResourceIsOverridenByBinding [8 ms]
  Passed BindingContext [< 1 ms]
  Passed DynResourceIsPreservedOnStyleValue [< 1 ms]
  Passed When the BindingContext changes, any bindings should be immediately applied. [< 1 ms]
  Passed StyleBindingIsOverridenByStyleDynResource [< 1 ms]
  Passed BindingContextBoundThroughConverter [3 ms]
[xUnit.net 00:00:02.72]       Assert.Equal() Failure: Values differ
[xUnit.net 00:00:02.72]       Expected: 42
[xUnit.net 00:00:02.72]       Actual:   0
[xUnit.net 00:00:02.72]       Stack Trace:
[xUnit.net 00:00:02.72]         /_/src/Controls/tests/Core.UnitTests/BindableObjectUnitTests.cs(1621,0): at Microsoft.Maui.Controls.Core.UnitTests.BindableObjectUnitTests.DefaultValueCreatorCachesValueWhenReentrantPropertyAddsResizeStore()
[xUnit.net 00:00:02.72]            at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
[xUnit.net 00:00:02.72]            at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
[xUnit.net 00:00:02.71]     DefaultValueCreatorCachesValueWhenReentrantPropertyAddsResizeStore [FAIL]
  Failed DefaultValueCreatorCachesValueWhenReentrantPropertyAddsResizeStore [5 ms]
  Error Message:
   Assert.Equal() Failure: Values differ
Expected: 42
Actual:   0
  Stack Trace:
     at Microsoft.Maui.Controls.Core.UnitTests.BindableObjectUnitTests.DefaultValueCreatorCachesValueWhenReentrantPropertyAddsResizeStore() in /_/src/Controls/tests/Core.UnitTests/BindableObjectUnitTests.cs:line 1621
   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
  Passed IsSetIsTrueWhenPropSetToDefault [< 1 ms]
  Passed PropertyChangedDefaultValue [< 1 ms]
  Passed TestBindingTwoWayOnReadOnly [1 ms]
  Passed BoundBindingContextUpdate [30 ms]
  Passed ParentAndChildBindingContextChanged [< 1 ms]
  Passed StyleValueIsOverridenByBinding [< 1 ms]
  Passed BoundBindingContextChange [1 ms]
  Passed SetBindingInvalid [< 1 ms]
  Passed StyleValueIsOverridenByValue [< 1 ms]
  Passed BindingContextGetter [1 ms]
  Passed GetValuesReturnsSetStateAndValue [< 1 ms]
  Passed ClearValue [1 ms]
  Passed RemovedBindingDoesNotUpdate [< 1 ms]
  Passed DefaultValueCreatorOnlyInvokedOnGetValue [< 1 ms]
  Passed SpecificityOfHandlers [< 1 ms]
  Passed GetValueDefault [< 1 ms]
  Passed DynResourceIsPreservedOnStyleDynResource [< 1 ms]
  Passed BindingContextChangedEvent [1 ms]
  Passed When an INPC implementer is unset as the BindingContext, its changes shouldn't be listened to any further. [1 ms]
  Passed BindingContextChangedOnce [< 1 ms]
  Passed StyleValueIsOverridenByStyleDynResource [< 1 ms]
  Passed When the BindingContext changes, the new context needs to listen for updates. [< 1 ms]
  Passed StyleValueIsOverridenByDynResource [< 1 ms]
  Passed DefaultValueCreatorCalledForChangeDelegates [< 1 ms]
  Passed ClearValueTriggersINPC [< 1 ms]
  Passed StyleBindingIsOverridenByDynResource [< 1 ms]
  Passed SetBindingToTextInvokesToString [< 1 ms]
  Passed StyleValueIsOverridenByStyleBinding [< 1 ms]
  Passed ClearValueInvalid [< 1 ms]
  Passed StyleDynResourceNotOverridenByStyleBinding [< 1 ms]
  Passed SetValueToTextInvokesToString [< 1 ms]
  Passed SetValueCoreImplicitelyCastBasicType [2 ms]
  Passed BindablePropertyChanged [< 1 ms]
  Passed DefaultValueCreatorNotSharedAccrossInstances [< 1 ms]
  Passed DefaultValueCreatorDoesNotTriggerINPC [< 1 ms]
  Passed DynResourceIsPreservedOnStyleBinding [< 1 ms]
  Passed StyleDynResourceIsOverridenByValue [< 1 ms]
  Passed StyleDynResourceIsOverridenByStyleValue [< 1 ms]
  Passed StyleDynResourceIsOverridenByDynResource [< 1 ms]
  Passed BindingIsPreservedOnStyleDynResource [< 1 ms]
  Passed ValueIsPreservedOnStyleValue [< 1 ms]
  Passed TestReadOnlyProperties [< 1 ms]
  Passed IsSetIsFalseWhenPropCleared [< 1 ms]
  Passed DoesNotRaiseOnSilentEvenWithRaiseOnEqual [< 1 ms]
  Passed GetValues [< 1 ms]
  Passed RemoveUnaddedBinding [< 1 ms]
  Passed IsSetIsFalseWhenPropNotSet [< 1 ms]
  Passed StyleDynResourceIsOverridenByStyleDynResource [< 1 ms]
  Passed ParentSetOnNullChildBindingContext [1 ms]
  Passed GetValueInvalid [< 1 ms]
  Passed ParentSetOnNonNullChildBindingContext [< 1 ms]
  Passed DefaultValueCreatorNotInvokedAfterClearValue [< 1 ms]
  Passed InvalidValueNotApplied [< 1 ms]
  Passed StyleBindingIsOverridenByBinding [< 1 ms]
  Passed BindingIsPreservedOnStyleValue [< 1 ms]
  Passed ValueIsPreservedOnStyleDynResource [< 1 ms]
  Passed TestBindingOneWayOnReadOnly [< 1 ms]
  Passed PropertyChangingSameValue [1 ms]
  Passed BindingsAppliedUnappliedWithNullContext [< 1 ms]
  Passed DefaultValueCreatorIsInvokedOnlyAtFirstTime [< 1 ms]
  Passed PropertyChanging [< 1 ms]
  Passed RemoveBindingInvalid [< 1 ms]
  Passed BindingIsPreservedOnStyleBinding [1 ms]
  Passed ClearValueDoesNotTriggersINPCOnSameValues [< 1 ms]
  Passed PropertyChangedSameValue [< 1 ms]
  Passed DoesNotRaiseOnSilent [< 1 ms]
  Passed StyleBindingIsOverridenByStyleValue [< 1 ms]
  Passed ValueIsPreservedOnStyleBinding [< 1 ms]
  Passed PropertyChangingDefaultValue [< 1 ms]
  Passed BindingOnBindingContextDoesntReapplyBindingContextBinding [1 ms]
  Passed GetSetValue [< 1 ms]
[xUnit.net 00:00:02.80]   Finished:    Microsoft.Maui.Controls.Core.UnitTests
  Passed IsSetIsTrueWhenPropSetByDefaultValueCreator [< 1 ms]
  Passed PropertyChanged [1 ms]
  Passed CoerceValue [< 1 ms]
  Passed StyleValueIsOverridenByStyleValue [< 1 ms]
  Passed BindingContextChangedCompareReferences [< 1 ms]

Test Run Failed.
Total tests: 95
     Passed: 94
     Failed: 1
 Total time: 3.5815 Seconds

🟢 With fix — 🧪 BindableObjectUnitTests: PASS ✅ · 108s

  Determining projects to restore...
  All projects are up-to-date for restore.
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14432623
  Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14432623
  Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14432623
  Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14432623
  Maps -> /home/vsts/work/1/s/artifacts/bin/Maps/Debug/net10.0/Microsoft.Maui.Maps.dll
  Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14432623
  Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14432623
  Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0/Microsoft.Maui.Controls.Maps.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14432623
  Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0/Microsoft.Maui.Controls.Xaml.dll
  TestUtils -> /home/vsts/work/1/s/artifacts/bin/TestUtils/Debug/netstandard2.0/Microsoft.Maui.TestUtils.dll
  Controls.Core.UnitTests -> /home/vsts/work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll
Test run for /home/vsts/work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (x64)

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:00.01] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 10.0.0)
[xUnit.net 00:00:00.55]   Discovering: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:04.95]   Discovered:  Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:04.98]   Starting:    Microsoft.Maui.Controls.Core.UnitTests
  Passed GetValuesDefaults [35 ms]
  Passed EventSubscribingOnBindingContextChanged [37 ms]
  Passed RecursiveChange [3 ms]
  Passed IsSetIsTrueWhenPropSet [< 1 ms]
  Passed DefaultValueCreator [7 ms]
  Passed SetValueCoreInvokesOpImplicitOnPropertyType [3 ms]
  Passed StyleBindingIsOverridenByValue [14 ms]
  Passed BindingsEditableAfterUnapplied [3 ms]
  Passed SetValueInvalid [2 ms]
  Passed StyleBindingIsOverridenByStyleBinding [1 ms]
  Passed RaiseOnEqual [3 ms]
  Passed SetValueCoreInvokesOpImplicitOnValue [< 1 ms]
  Passed StyleDynResourceIsOverridenByBinding [10 ms]
  Passed BindingContext [< 1 ms]
  Passed DynResourceIsPreservedOnStyleValue [1 ms]
  Passed When the BindingContext changes, any bindings should be immediately applied. [< 1 ms]
  Passed StyleBindingIsOverridenByStyleDynResource [3 ms]
  Passed BindingContextBoundThroughConverter [3 ms]
  Passed DefaultValueCreatorCachesValueWhenReentrantPropertyAddsResizeStore [< 1 ms]
  Passed IsSetIsTrueWhenPropSetToDefault [< 1 ms]
  Passed PropertyChangedDefaultValue [1 ms]
  Passed TestBindingTwoWayOnReadOnly [< 1 ms]
  Passed BoundBindingContextUpdate [54 ms]
  Passed ParentAndChildBindingContextChanged [2 ms]
  Passed StyleValueIsOverridenByBinding [< 1 ms]
  Passed BoundBindingContextChange [3 ms]
  Passed SetBindingInvalid [< 1 ms]
  Passed StyleValueIsOverridenByValue [< 1 ms]
  Passed BindingContextGetter [2 ms]
  Passed GetValuesReturnsSetStateAndValue [1 ms]
  Passed ClearValue [1 ms]
  Passed RemovedBindingDoesNotUpdate [2 ms]
  Passed DefaultValueCreatorOnlyInvokedOnGetValue [< 1 ms]
  Passed SpecificityOfHandlers [< 1 ms]
  Passed GetValueDefault [< 1 ms]
  Passed DynResourceIsPreservedOnStyleDynResource [< 1 ms]
  Passed BindingContextChangedEvent [2 ms]
  Passed When an INPC implementer is unset as the BindingContext, its changes shouldn't be listened to any further. [1 ms]
  Passed BindingContextChangedOnce [< 1 ms]
  Passed StyleValueIsOverridenByStyleDynResource [< 1 ms]
  Passed When the BindingContext changes, the new context needs to listen for updates. [< 1 ms]
  Passed StyleValueIsOverridenByDynResource [< 1 ms]
  Passed DefaultValueCreatorCalledForChangeDelegates [2 ms]
  Passed ClearValueTriggersINPC [< 1 ms]
  Passed StyleBindingIsOverridenByDynResource [< 1 ms]
  Passed SetBindingToTextInvokesToString [2 ms]
  Passed StyleValueIsOverridenByStyleBinding [2 ms]
  Passed ClearValueInvalid [< 1 ms]
  Passed StyleDynResourceNotOverridenByStyleBinding [< 1 ms]
  Passed SetValueToTextInvokesToString [< 1 ms]
  Passed SetValueCoreImplicitelyCastBasicType [4 ms]
  Passed BindablePropertyChanged [< 1 ms]
  Passed DefaultValueCreatorNotSharedAccrossInstances [< 1 ms]
  Passed DefaultValueCreatorDoesNotTriggerINPC [< 1 ms]
  Passed DynResourceIsPreservedOnStyleBinding [< 1 ms]
  Passed StyleDynResourceIsOverridenByValue [< 1 ms]
  Passed StyleDynResourceIsOverridenByStyleValue [< 1 ms]
  Passed StyleDynResourceIsOverridenByDynResource [< 1 ms]
  Passed BindingIsPreservedOnStyleDynResource [2 ms]
  Passed ValueIsPreservedOnStyleValue [< 1 ms]
  Passed TestReadOnlyProperties [2 ms]
  Passed IsSetIsFalseWhenPropCleared [< 1 ms]
  Passed DoesNotRaiseOnSilentEvenWithRaiseOnEqual [< 1 ms]
  Passed GetValues [< 1 ms]
  Passed RemoveUnaddedBinding [< 1 ms]
  Passed IsSetIsFalseWhenPropNotSet [2 ms]
  Passed StyleDynResourceIsOverridenByStyleDynResource [< 1 ms]
  Passed ParentSetOnNullChildBindingContext [< 1 ms]
  Passed GetValueInvalid [3 ms]
  Passed ParentSetOnNonNullChildBindingContext [< 1 ms]
  Passed DefaultValueCreatorNotInvokedAfterClearValue [< 1 ms]
  Passed InvalidValueNotApplied [< 1 ms]
  Passed StyleBindingIsOverridenByBinding [2 ms]
  Passed BindingIsPreservedOnStyleValue [< 1 ms]
  Passed ValueIsPreservedOnStyleDynResource [< 1 ms]
  Passed TestBindingOneWayOnReadOnly [< 1 ms]
  Passed PropertyChangingSameValue [6 ms]
  Passed BindingsAppliedUnappliedWithNullContext [< 1 ms]
  Passed DefaultValueCreatorIsInvokedOnlyAtFirstTime [< 1 ms]
  Passed PropertyChanging [< 1 ms]
  Passed RemoveBindingInvalid [< 1 ms]
  Passed BindingIsPreservedOnStyleBinding [< 1 ms]
  Passed ClearValueDoesNotTriggersINPCOnSameValues [< 1 ms]
  Passed PropertyChangedSameValue [< 1 ms]
  Passed DoesNotRaiseOnSilent [< 1 ms]
  Passed StyleBindingIsOverridenByStyleValue [< 1 ms]
  Passed ValueIsPreservedOnStyleBinding [< 1 ms]
  Passed PropertyChangingDefaultValue [< 1 ms]
  Passed BindingOnBindingContextDoesntReapplyBindingContextBinding [< 1 ms]
  Passed GetSetValue [< 1 ms]
[xUnit.net 00:00:05.42]   Finished:    Microsoft.Maui.Controls.Core.UnitTests
  Passed IsSetIsTrueWhenPropSetByDefaultValueCreator [< 1 ms]
  Passed PropertyChanged [< 1 ms]
  Passed CoerceValue [< 1 ms]
  Passed StyleValueIsOverridenByStyleValue [< 1 ms]
  Passed BindingContextChangedCompareReferences [< 1 ms]

Test Run Successful.
Total tests: 95
     Passed: 95
 Total time: 6.8253 Seconds

📁 Fix files reverted (1 files)

• src/Controls/src/Core/BindableObject.cs

---

Pre-Flight — Context & Validation

Issue: #33584 - BindableObject property access micro-optimizations
PR: #36030 - [Controls] Fix BindableObject DefaultValueCreator re-entrancy regression
Platforms Affected: all platforms (core Controls bindable-property infrastructure); testing platform: android
Files Changed: 1 implementation, 1 test

Key Findings

• PR fixes a regression introduced by #33584 where CollectionsMarshal.GetValueRefOrAddDefault held a ref into _properties while CreateContext invoked DefaultValueCreator; re-entrant SetValue calls could resize the dictionary and leave the live slot unset/null.
• Current PR approach removes the ref-based add path and uses GetContext -> CreateContext -> _properties.Add, matching the previously safe NETSTANDARD branch shape.
• Gate was already completed before this phase: the regression test fails without the fix and passes with the PR fix. Gate artifacts were preserved and not overwritten.
• Changed tests are unit tests in src/Controls/tests/Core.UnitTests/BindableObjectUnitTests.cs; no UI test category is directly impacted.
• GitHub CLI is unauthenticated in this environment, so pre-flight used public GitHub API/web fetch plus local checked-out PR branch data.

Code Review Summary

Verdict: NEEDS_DISCUSSION
Confidence: low
Errors: 0 | Warnings: 0 | Suggestions: 1

Key code review findings:

• 💡 Optional readability suggestion: GetValuesReturnsSetStateAndValue could clarify why an unset property with property default 1 returns Value == 0 from GetValues<int>.
• Blast radius: BindableObject.GetOrCreateContext is shared, startup-adjacent, hot-path infrastructure used by all BindableObject instances; no static/shared state is introduced.
• Failure mode: re-entrant default-value creation for different properties is fixed because no dictionary value ref is held across CreateContext.
• CI status from independent review was pending/undetermined for required PR checks, so the code-review verdict was capped to NEEDS_DISCUSSION despite no blocking code findings.

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #36030	Replace CollectionsMarshal.GetValueRefOrAddDefault in BindableObject.GetOrCreateContext with safe two-step lookup/create/add so no dictionary ref is held across re-entrant DefaultValueCreator execution.	✅ PASSED (Gate)	src/Controls/src/Core/BindableObject.cs, src/Controls/tests/Core.UnitTests/BindableObjectUnitTests.cs	Original PR

---

Code Review — Deep Analysis

Code Review — PR #36030

Independent Assessment

What this changes: Removes the CollectionsMarshal.GetValueRefOrAddDefault-based fast path from GetOrCreateContext in BindableObject. The #if NETSTANDARD / #else conditional is eliminated; all TFMs now use the original NETSTANDARD-style two-stage pattern: GetContext (TryGetValue) -> CreateContext -> _properties.Add. Also adds two new unit tests and removes the now-unused using System.Runtime.InteropServices.

Inferred motivation: GetValueRefOrAddDefault returns a ref into the dictionary's backing array. If CreateContext runs a DefaultValueCreator that re-entrantly calls SetValue on other properties, those insertions can grow _properties past capacity, reallocating the backing array. The stale ref then writes through to the discarded array; the live dictionary slot stays null, causing the second GetValue call to fall back to property.DefaultValue with IsSet == false.

Is the approach sound? Yes. The two-stage pattern is immune to this class of invalidation because no ref is held across the potentially-resizing CreateContext call. _properties.Add is issued after the creator returns, targeting whatever the current post-resize array is. The only performance delta is one additional TryGetValue miss on the cold path; the hot path where a context already exists remains a dictionary lookup.

Reconciliation with PR Narrative

Author claims: Regression fix for #33584. Uses the #if NETSTANDARD branch's shape as the fix. Explains why this is not a straight revert: BindableProperty.InternalId and GetValues<T> are now consumed by other code. Tests two new cases: the regression scenario and direct coverage for GetValues<T>.

Agreement/disagreement: Full agreement. The root-cause analysis is accurate: ref invalidation under dictionary resize is a real hazard with CollectionsMarshal.GetValueRefOrAddDefault. The two-test approach is appropriate. One nuance worth noting: GetValuesReturnsSetStateAndValue expects values[1].Value == 0 for prop1 even though the property default is 1; this is intentional because GetValues<T> returns stored-only values, not computed defaults.

Prior Review Reconciliation

Prior ❌ Error Finding	Source	Status	Evidence
—	—	—	—

No prior ❌ Error findings found.

Blast Radius Assessment

• Runs for all instances: Yes. GetOrCreateContext is on the critical path of GetValue, SetValue, and binding operations on BindableObject.
• Startup impact: Yes. Property initialization can happen during startup. The change restores the safe branch shape already used for NETSTANDARD.
• Static/shared state: No. _properties is per-instance.

Blast radius classification: shared infrastructure / hot path, so confidence is capped at low.

CI Status

• Required-check result: pending/undetermined from the independent review environment; local gh pr checks could not be used in this orchestrator because gh is unauthenticated.
• Classification: undetermined for full PR CI; gate result supplied by the caller is ✅ PASSED for the regression tests.
• Action taken: confidence capped at low.

Findings

💡 Suggestion — Clarify stored-value semantics in GetValuesReturnsSetStateAndValue

src/Controls/tests/Core.UnitTests/BindableObjectUnitTests.cs sets prop1 with defaultValue: 1 and expects values[1].Value == 0. A short inline comment could protect this correct assertion from future confusion: GetValues<T> returns stored values plus set-state, not property defaults for unset contexts.

Failure-Mode Probing

• What if DefaultValueCreator does not trigger a resize? GetContext misses, CreateContext runs, then _properties.Add inserts the new context. Correct.
• What if DefaultValueCreator re-entrantly sets other properties and resizes _properties? The new code holds no ref across CreateContext, so insertion after the creator returns targets the current live dictionary storage. Correct.
• What if DefaultValueCreator re-entrantly accesses the same property? Old and new behavior are both pathological; a self-referential creator can recurse or cause duplicate insertion. This is not introduced by the PR.
• Handler disconnect/reconnect: no direct interaction; _properties is per-instance and independent of handler lifecycle.
• Null safety: CreateContext returns a non-null BindablePropertyContext; _properties.Add is reached only when GetContext returned null in normal flow.

Verdict: NEEDS_DISCUSSION

Confidence: low
Summary: The code fix is correct, minimal, and well-tested for the regression. No ❌ Error or ⚠️ Warning findings were identified. The verdict is capped to NEEDS_DISCUSSION because this is shared bindable-property infrastructure and full required CI status was not available from the unauthenticated local environment.

---

Fix — Analysis & Comparison

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
1	try-fix-1	Keep CollectionsMarshal.GetValueRefOrAddDefault, but pre-call _properties.EnsureCapacity(_properties.Count + 1).	❌ FAIL	1 file	Still fails with Expected: 42 Actual: 0; capacity for one slot does not cover re-entrant additions inside DefaultValueCreator.
2	try-fix-2	Insert placeholder BindablePropertyContext into _properties before running DefaultValueCreator, then initialize it in place.	✅ PASS	1 file	Passes focused tests, but exposes a partially initialized context during default creator execution; not better than PR.
3	try-fix-3	Hybrid: safe path for properties with DefaultValueCreator, retain CollectionsMarshal for static-default properties.	✅ PASS	1 file	Passes focused tests, but reintroduces complexity for a marginal cold write-path optimization; not better than PR.
4	try-fix-4	Use GetValueRefOrAddDefault for slot creation, then assign final context through _properties[property.InternalId] after creator returns.	✅ PASS	1 file	Passes focused tests, but leaves a null placeholder visible during creator execution; not better than PR.
PR	PR #36030	Create context before adding it to _properties, holding no dictionary value ref across DefaultValueCreator.	✅ PASSED (Gate)	2 files	Original PR

Cross-Pollination

Model	Round	New Ideas?	Details
maui-expert-reviewer	1	Yes	Candidate 1 tested despite predicted invalidity; failure confirms capacity management with one extra slot is insufficient.
maui-expert-reviewer	1	Yes	Candidate 2 passes but is semantically weaker because the context is observable before it is fully initialized.
maui-expert-reviewer	1	Yes	Candidate 3 passes but is a maintainability/performance tradeoff, not a better correctness fix.
maui-expert-reviewer	1	Yes	Candidate 4 passes but keeps a null-placeholder window and ref-based complexity.
maui-expert-reviewer	2	No	NO NEW IDEAS. The design space is exhausted: avoid holding refs across creator, prevent resize, insert placeholder before creator, or change data structure. The PR is the cleanest version of the only robust category.

Exhausted: Yes
Selected Fix: PR #36030 — The PR fix is the best candidate. It is the only passing approach that avoids both stale dictionary refs and partially initialized/null context visibility during DefaultValueCreator execution, while also simplifying the code by removing the CollectionsMarshal/TFM split.

---

Recommended PR Title & Description

Recommended title

[Controls] Fix BindableObject DefaultValueCreator reentrancy

Recommended description

Fix a BindableObject regression where a DefaultValueCreator that re-entrantly sets other properties could resize the property store and prevent the created default context from being cached.

- Create the BindablePropertyContext before adding it to _properties, avoiding a held dictionary value ref across DefaultValueCreator execution.
- Remove the now-unneeded CollectionsMarshal-based TFM split.
- Add unit coverage for the re-entrant resize regression and GetValues<T> set-state/value behavior.

---

Report — Final Recommendation

Comparative Fix Report — PR #36030

Candidate ranking

Rank	Candidate	Regression result	Assessment
1	pr	✅ PASS	Best candidate. It creates the context before inserting into _properties, so no dictionary value ref or placeholder is observable across DefaultValueCreator execution. It is the simplest passing fix and removes the TFM split/ref-based complexity.
2	pr-plus-reviewer	✅ PASS	Equivalent to pr; the expert reviewer produced no actionable findings, so applying reviewer feedback creates no code delta. Ranked after pr only because it does not improve on the submitted fix.
3	try-fix-3	✅ PASS	Correct for the tested regression and preserves CollectionsMarshal for static-default properties, but adds branching and keeps an optimization whose benefit is marginal because simple-default reads do not need context creation. More complex than the PR.
4	try-fix-2	✅ PASS	Avoids stale refs by inserting a placeholder object before default creation, but exposes a partially initialized context during DefaultValueCreator execution. That semantic risk makes it weaker than the PR.
5	try-fix-4	✅ PASS	Fixes final assignment through the live dictionary, but still inserts a null placeholder before DefaultValueCreator runs. Same-property re-entrant access can observe the placeholder window, so it is weaker than the PR.
6	try-fix-1	❌ FAIL	Fails the regression test (Expected: 42, Actual: 0). Ensuring capacity for one slot does not protect against multiple re-entrant additions and dictionary resize. Per ranking rules, it must be below all passing candidates.

Winning candidate

pr wins. It is the only candidate that is both passing and avoids all known semantic hazards: no stale dictionary ref, no null/partial placeholder context, no extra branch complexity, and no reliance on capacity guesses.

Comparison notes

• pr and pr-plus-reviewer are identical because the expert reviewer found no actionable inline issues.
• The passing try-fix candidates solve the focused regression but either preserve unnecessary CollectionsMarshal complexity (try-fix-3) or make the target property observable in _properties before it is fully initialized (try-fix-2, try-fix-4).
• The failing candidate (try-fix-1) is disqualified below every passing candidate because it does not satisfy the regression gate.

---

Future Action — review latest findings

No alternative fix was selected for this run. Review the session findings and CI results before merging.

[kubaflo]

Note

🤖 This review was automatically generated by a multi-model AI review system (Claude Opus 4.8, GPT-5.5, Gemini 3.1 Pro). Three models independently reviewed the code, then cross-pollinated their findings to produce this consolidated review.

Multi-Model Review — Round 1

Verdict: LGTM

After independent review and cross-pollination, all three models converged on LGTM with high confidence:

• Gemini: LGTM (high confidence)
• GPT: LGTM (medium → high confidence after cross-pollination)
• Opus: LGTM (high confidence)

---

Summary

This PR fixes a use-after-realloc regression from #33584. The bug: GetOrCreateContext held a ref from CollectionsMarshal.GetValueRefOrAddDefault across CreateContext, which can re-entrantly call SetValue on other properties, resizing _properties and invalidating the ref. Result: created context written to discarded array, never stored.

The fix: Create context first (holding no ref), then Add it — restoring the original pre-#33584 shape.

---

What All Models Verified

✅ Root cause confirmed: Re-entrant dictionary resize invalidates CollectionsMarshal.GetValueRefOrAddDefault ref
✅ Fix covers both paths: Read (GetValue) and write (SetValue) code paths
✅ Performance preserved: Hot path unchanged (single TryGetValue); cold path adds one-time probe
✅ Thread-safety unchanged: Single-threaded re-entrancy; _properties never thread-safe by design
✅ Exception safety bonus: Fix restores rollback semantics if CreateContext throws
✅ No public API change: Correctly no PublicAPI.Unshipped.txt entry
✅ Tests validate the regression: New tests reproduce the bug and confirm the fix
✅ Consumer audit (Opus): ShellAppearance gates all .Value access behind .IsSet — documented default(T) for unset is harmless
✅ CI passes: Required checks green

---

Cross-Pollination Insights

• Unanimous convergence: All 3 models independently found zero inline issues
• GPT upgraded confidence (medium → high) after reading deeper analyses from Opus and Gemini
• Gemini independently reached Opus's exception-safety observation
• No conflicts: All JSON arrays empty, all verdicts LGTM

---

Confidence Assessment

High — Perfect 3/3 consensus with independent verification of root cause, fix correctness, performance profile, exception safety, and blast radius. The fix is minimal, correct, and well-tested.

---

[sheiksyedm]

@kubaflo This fix is not required and it doesn't fixed the actual regression. We already created a new PR #36063 that address the actual regression. So, you can close this PR.