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

NullReferenceException when composing the CSharp.Features assembly #11841

Closed
aelij opened this issue Jun 8, 2016 · 15 comments
Closed

NullReferenceException when composing the CSharp.Features assembly #11841

aelij opened this issue Jun 8, 2016 · 15 comments
Labels
Area-IDE Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented

Comments

@aelij
Copy link
Contributor

aelij commented Jun 8, 2016

Version Used: 2.0.0-beta1-20160603-02

Steps to Reproduce:

  1. Create host services with the CSharp.Features assembly:
MefHostServices.Create(MefHostServices.DefaultAssemblies.Concat(new[] {
    Assembly.Load("Microsoft.CodeAnalysis.Features"), 
    Assembly.Load("Microsoft.CodeAnalysis.CSharp.Features") }))

Expected Behavior:
No exception

Actual Behavior:

System.NullReferenceException Object reference not set to an instance of an object.
   at System.Composition.TypedParts.Discovery.TypeInspector.AddMetadata(IDictionary`2 metadata, String name, Object value)
   at System.Composition.TypedParts.Discovery.TypeInspector.ReadMetadataAttribute(Attribute attribute, IDictionary`2 metadata)
   at System.Composition.TypedParts.Discovery.TypeInspector.ReadLooseMetadata(Object[] appliedAttributes, IDictionary`2 metadata)
   at System.Composition.TypedParts.Discovery.TypeInspector.<DiscoverInstanceExports>d__9.MoveNext()
   at System.Composition.TypedParts.Discovery.TypeInspector.<DiscoverExports>d__0.MoveNext()
   at System.Composition.TypedParts.Discovery.TypeInspector.InspectTypeForPart(TypeInfo type, DiscoveredPart& part)
   at System.Composition.TypedParts.TypedPartExportDescriptorProvider..ctor(IEnumerable`1 types, AttributedModelProvider attributeContext)
   at System.Composition.Hosting.ContainerConfiguration.CreateContainer()
   at Microsoft.CodeAnalysis.Host.Mef.MefHostServices.Create(IEnumerable`1 assemblies)

Note that this is probably related to this PR: dotnet/corefx#6605
For example, GenerateMethodCodeFixProvider has two ExtensionOrder attributes.

Workaround:
Skip ExtensionOrder attributes:

var compositionContext = new System.Composition.Hosting.ContainerConfiguration()
    .WithAssemblies(MefHostServices.DefaultAssemblies.Concat(new[] { 
        Assembly.Load("Microsoft.CodeAnalysis.Features"), Assembly.Load("Microsoft.CodeAnalysis.CSharp.Features") }))
    .WithDefaultConventions(new AttributeFilterProvider())
    .CreateContainer();

var host = MefHostServices.Create(compositionContext);

private class AttributeFilterProvider : AttributedModelProvider
{
    public override IEnumerable<Attribute> GetCustomAttributes(Type reflectedType, MemberInfo member)
    {
        var customAttributes = member.GetCustomAttributes().Where(x => !(x is ExtensionOrderAttribute)).ToArray();
        return customAttributes;
    }

    public override IEnumerable<Attribute> GetCustomAttributes(Type reflectedType, ParameterInfo member)
    {
        var customAttributes = member.GetCustomAttributes().Where(x => !(x is ExtensionOrderAttribute)).ToArray();
        return customAttributes;
    }
}
@gafter gafter added the Area-IDE label Jun 8, 2016
@weltkante
Copy link
Contributor

Also affects the master branch, I'm running into this when trying to consume new CompletionService API.

@DustinCampbell
Copy link
Member

So, unless I'm missing something, this is a bug in MEF that has been fixed by @aelij (nice!) and Roslyn just needs to pick up a new System.Composition. Is that correct?

@weltkante
Copy link
Contributor

Yes, thats what its looking like currently. No idea if there are new nuget packages available for MEF yet.

@DustinCampbell
Copy link
Member

Hmmm... I'm trying to understand why I this was working for me in OmniSharp where I successful composed the Features assemblies. It looks like we're using Microsoft.Composition, 1.0.27 in Roslyn and OmniSharp is using Microsoft.Composition, 1.0.30. If you reference 1.0.30 in your app, does that address the problem?

@weltkante
Copy link
Contributor

weltkante commented Jun 24, 2016

1.0.30 doesn't seem to make a difference. I'm using code similar to the original post to load the assemblies, and Roslyn is compiled from master instead of using nuget packages.

@DustinCampbell
Copy link
Member

Hmmm... OK. The other big difference is that OmniSharp is using the 1.3.0 Roslyn packages rather than 2.0.0.

@stephentoub, @dsplaisted -- this looks related to dotnet/corefx#6605. Has that made it into any official NuGet package yet? It looks like the last Microsoft.Composition release was 1.0.30, which is a year and a half ago.

@stephentoub
Copy link
Member

Has that made it into any official NuGet package yet? It looks like the last Microsoft.Composition release was 1.0.30, which is a year and a half ago.

@mellinoe has a PR adding packages for System.Composition.* here:
dotnet/corefx#9640
I'm not sure about Microsoft.Composition.

@DustinCampbell
Copy link
Member

OK. So, this is in flux at the moment. @tmat, you're our portability guru, right? 😄 Any thoughts?

Looking at GenerateMethodCodeFixProvider, it seems like having multiple ExtensionOrder attributes is unnecessary and they could be combined into one. I wonder if that's the case everywhere?

cc @jasonmalinowski, @Pilchie

@weltkante
Copy link
Contributor

GenerateMethodCodeFixProvider seems to be the only one preventing MEF to load the assembly. Merging both attributes into one and recompiling CSharp.Features allows MEF to load it 👍

@DustinCampbell
Copy link
Member

I see one other but it's in the Editor Features layer. I also see others that have combined the Before and After values, so I'm confident there's not some underlying bug. Feel like submitting a PR @weltkante?

@weltkante
Copy link
Contributor

weltkante commented Jun 24, 2016

I can try making one, not very fluent with github yet.

@DustinCampbell DustinCampbell added the Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented label Jun 24, 2016
@DustinCampbell
Copy link
Member

@weltkante's PR has been merged, which should workaround this issue for the Features layer.

@mattwar
Copy link
Contributor

mattwar commented Jun 24, 2016

we should have a test that successfully composes the features layer into an AdHocWorkspace.

@DustinCampbell
Copy link
Member

You're right. We should. I'll do that.

@aelij
Copy link
Contributor Author

aelij commented Jun 24, 2016

Cool :)
@DustinCampbell maybe you should also open an issue about upgrading to System.Composition once it's out on NuGet?

DustinCampbell added a commit to DustinCampbell/roslyn that referenced this issue Jun 24, 2016
Issue dotnet#11841 found that, due to a MEF bug, the Features assemblies could not properly be included in a MEF composition. This was worked around by combining [ExtensionOrder] attributes where multiple attributes had been specified. This change adds a tests for C# and VB to ensure that a CompletionService can be acquired from an AdhocWorkspace that is created with MefHostServices that include the Features assemblies.

I verified that these tests fail without the workaound described above, and pass with the workaround in place.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented
Projects
None yet
Development

No branches or pull requests

6 participants