Skip to content
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.

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

Stack overflow exception when creating and using multiple instances of XamlDirective in parallel #10313

Open
ThomasGoulet73 opened this issue Jan 18, 2025 · 1 comment · May be fixed by #10314
Assignees
Labels
Bug Product bug (most likely) 🚧 work in progress

Comments

@ThomasGoulet73
Copy link
Contributor

Description

This bug isn't really specific to XamlDirective but it's the class where I found the issue and it's the easiest to cause an issue. You can have the bug when using instances of XamlMember or of any of its descendants (In this case XamlDirective).

The bug is that XamlMember._reflector can be wrongly initialized when hitting this point from 3 threads and with very specific timing:

_reflector = reflector ?? MemberReflector.UnknownReflector;

Here's the code for MemberReflector.UnknownReflector:

internal static MemberReflector UnknownReflector
{
get
{
if (s_UnknownReflector is null)
{
s_UnknownReflector = new MemberReflector
{
_designerSerializationVisibility = DesignerSerializationVisibility.Visible,
_memberBits = (int)BoolMemberBits.Default |
(int)BoolMemberBits.Unknown | (int)BoolMemberBits.AllValid
};
// Explicitly set all the nullable references so that IsSet is true
s_UnknownReflector._deferringLoader.Value = null;
s_UnknownReflector._getter.Value = null;
s_UnknownReflector._setter.Value = null;
s_UnknownReflector._typeConverter.Value = null;
s_UnknownReflector._valueSerializer.Value = null;
s_UnknownReflector.DependsOn = XamlType.EmptyList<XamlMember>.Value;
s_UnknownReflector.Invoker = XamlMemberInvoker.UnknownInvoker;
s_UnknownReflector.Type = XamlLanguage.Object;
}
return s_UnknownReflector;
}
}

Here's the required timing of those 3 threads to have an invalid MemberReflector returned from MemberReflector.UnknownReflector:
Thread 1 and 2 invoke MemberReflector.UnknownReflector and find that s_UnknownReflector is null. Thread 1 assigns s_UnknownReflector to a new instance. Thread 3 invokes MemberReflector.UnknownReflector, finds that s_UnknownReflector is not null and returns s_UnknownReflector. Thread 2 assigns s_UnknownReflector to a new instance.

In this scenario thread 1 and 2 both return the same instance from MemberReflector.UnknownReflector while thread 3 returned an old instance from MemberReflector.UnknownReflector that wasn't fully initialized.



Now that I've explained how to have a partially initialized MemberReflector, here's how to have a stack overflow exception using XamlDirective:
We enter XamlDirective.Type when _reflector.Type is null which means that we enter this if block:

if (_reflector.Type is null)
{
_reflector.Type = LookupType() ?? XamlLanguage.Object;
}

In the if block it calls the virtual method XamlDirective.LookupType() which is implemented in XamlDirective to return base.Type:

protected sealed override XamlType LookupType()
{
// We set this value in ctor, so this call won't produce infinite loop
return base.Type;
}

We then have an infinite loop of Type -> LookupType() -> Type -> LookupType() -> etc.

Reproduction Steps

Add this code to https://github.com/dotnet/wpf/blob/b08d70d354d9f0f34ecf4bcd5f214e57dd5dab1e/src/Microsoft.DotNet.Wpf/tests/UnitTests/System.Xaml.Tests/System/Xaml/XamlDirectiveTests.cs

[Theory]
[InlineData("xamlNamespace", "name")]
public void Parallel_Ctor_String_String(string xamlNamespace, string? name)
{
    Parallel.For(0, 1000, i =>
    {
        var directive = new XamlDirective(xamlNamespace, name);
        Assert.Equal(XamlLanguage.Object, directive.Type);
    });
}

Run the test a couple of times and it should crash the process with a stack overflow exception.

Expected behavior

The process shouldn't crash with a stack overflow exception.

Actual behavior

The process crashes with a stack overflow exception.

Regression?

No.

Known Workarounds

None.

Impact

Can cause the process to crash or weird bugs at runtime but the chance of getting this issue in practice is extremely low. As explained in the description, it requires 3 threads with very specific timing so I think it would be very hard to hit in practice.

Configuration

.Net 9.0
Windows 11
x64
Not specific to that configuration.

Other information

No response

@ThomasGoulet73
Copy link
Contributor Author

I hit this issue in CI while working on my PR #10304 and I was able to track down the problematic code.

I have a fix ready that I'll send in a minute.

I'm assigning this issue to myself.

@ThomasGoulet73 ThomasGoulet73 added the Bug Product bug (most likely) label Jan 18, 2025
@ThomasGoulet73 ThomasGoulet73 self-assigned this Jan 18, 2025
ThomasGoulet73 added a commit to ThomasGoulet73/wpf that referenced this issue Jan 18, 2025
@ThomasGoulet73 ThomasGoulet73 linked a pull request Jan 18, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Product bug (most likely) 🚧 work in progress
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant