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

Roslyn Compilation, 160 errors and process terminated due to NullReferenceException when a class with primary constructor implements INotifyPropertyChanged #71400

Closed
sherif-elmetainy opened this issue Dec 22, 2023 · 2 comments · Fixed by #71497

Comments

@sherif-elmetainy
Copy link

sherif-elmetainy commented Dec 22, 2023

Version Used:
C# 12, dotnet SDK version 8.0.100, MSBuild version 17.8.3+195e7f5a3 for .NET, Visual Studio Professional 2022 Version 17.8.3
Also occurs when using Rider version 2023.3.2

Steps to Reproduce:
1- Create a C# file with the following code and try to compile it.

using System.ComponentModel;

internal class MyClass(MyOtherClass otherClass, string key, int value) : INotifyPropertyChanged
{
    public string MyProperty { get; private set; } = GetValue(otherClass.MyProperty, key, value);
    private PropertyChangedEventHandler? _propertyChanged;


    private static string GetValue(string myProperty, string key, int value)
    {
        throw new NotImplementedException();
    }

    public event PropertyChangedEventHandler? PropertyChanged
    {
        add
        {
            lock (this)
            {
                if (_propertyChanged is null)
                {
                    otherClass.SomethingChanged += OnSomethingChanged;
                }
                _propertyChanged += value;

            }
        }
        remove
        {
            lock (this)
            {
                if (_propertyChanged is null)
                {
                    return;
                }
                _propertyChanged -= value;
                if (_propertyChanged is null)
                {
                    otherClass.SomethingChanged -= OnSomethingChanged;
                }
            }
        }
    }

    private void OnSomethingChanged(object? sender, EventArgs e)
    {
        throw new NotImplementedException();
    }
}

internal class MyOtherClass
{
    public event EventHandler? SomethingChanged;
    public string MyProperty { get; set; }
}

Expected Behavior:
The code should compile without error.

Actual Behavior:

  • When building from the command line (dotnet build), the C# compiler produces 160 errors the crashes with error C:\Program Files\dotnet\sdk\8.0.100\Roslyn\Microsoft.CSharp.Core.targets(84,5): error : Process terminated. System.NullReferenceException: Object reference not set to an instance of an object. [D:\source\my\ConsoleApp1\ConsoleApp1\ConsoleApp1.csproj] (This is a partial log of the error which is very big, the entire error log is attached here).
  • When building in Visual Studio I get C:\Program Files\Microsoft Visual Studio\2022\Professional\MSBuild\Current\Bin\Roslyn\Microsoft.CSharp.Core.targets(84,5): error MSB6006: "csc.exe" exited with code -2146232797.

Moreover, when working with either Visual Studio or Rider, the C# analyzers and generators stop working (apparently because of the Roslyn process crashing). While this may not be apparent in my sample reproduction, it occurs in my actual project, but I kept the reproduction to minimum and excluded irrelevant code.

This seems to be similar to #50198, however the old issue is resolved (may be this is a regression?) and old one was related to records and not primary constructors.

Workaround:

Using a normal constructor instead of a primary constructor with that class fixed the issue and allowed me to continue my work.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Dec 22, 2023
@jhinder
Copy link
Contributor

jhinder commented Dec 22, 2023

The repro can be simplified.

  • The event must be an interface implementation.
  • One of the event accessors needs to reference a field from the primary constructor, but it can be of any type.
  • The mere reference of the constructor parameter is enough to trigger the error.
using System;

interface I
{
    event EventHandler E;
}

class C(int i) : I
{
    public event EventHandler E
    {
        add { i }
        remove { }
    }
}

https://sharplab.io/#v2:EYLgtghglgdgPgAQEwEYCwAoTsAuBTAJwDMIBjPAAgElMBvTCxivANzxhwoFE2OAJCDAAmAG0LcA3JgC+mTMgoBhABS4KUAJQUQ1OgyYIAzM16ce7HAOFiC3fY3oYmzihCFCKtdRVlOXjAjwwAHs2Tx97CIxpIA=

In the cases where it doesn't throw an exception, it fails this assertion here:

@jaredpar jaredpar added Bug New Feature - Primary Constructors and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Jan 3, 2024
@jaredpar jaredpar added this to the 17.10 milestone Jan 3, 2024
AlekseyTs added a commit to AlekseyTs/roslyn that referenced this issue Jan 4, 2024
…process of figuring out its type.

This allows us to perform lookup operations within the accessor without triggering the process of
determining whether the event is a WINMD event. That process could request the list of containing type
members while we are building the list, thus causing an unexpected circularity and an unexpected reentrancy
into  SourceMemberMethodSymbol.LazyMethodChecks on the same thread.

Fixes dotnet#71400.
@nvmkpk
Copy link

nvmkpk commented Jan 13, 2024

This also happens in MAUI and WPF when implementing ICommand interface

AlekseyTs added a commit that referenced this issue Jan 13, 2024
…process of figuring out its type. (#71497)

This allows us to perform lookup operations within the accessor without triggering the process of
determining whether the event is a WINMD event. That process could request the list of containing type
members while we are building the list, thus causing an unexpected circularity and an unexpected reentrancy
into  SourceMemberMethodSymbol.LazyMethodChecks on the same thread.

Fixes #71400.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment