StackOverflowException in Microsoft.Framework.Runtime.LibraryManager.Flatten adding erroneous packages to KProject

[MattGal]

While experimenting with K projects, I added a reference to "System.Net.Http": "2.0.20710.0" from Nuget.Org. I expected this was going to have some errors, but not a constantly crashing klr.exe.

As soon as you add this reference, klr will crash repeatedly due to stack overflow in Microsoft.Framework.Runtime.LibraryManager.Flatten

[muratg]

@MattGal Thanks for reporting the issue. I was able to reproduce the problem and we'll investigate.

Looks like System.Net.Http is being renamed to Microsoft.Net.Http, and they are using System.Net.Http as a sort of a meta package. If you need to work around this issue, please reference Microsoft.Net.Http directly.

[muratg]

An additional bug exists in KPM.

In Matt's repro (where you have System.Net.Http as a dependency in your project.json):

• If you don't have a local Microsoft.Net.Http in your .kpm folder, kpm restore happily installs it.
• If you already have Microsoft.Net.Http there kpm throws the following error:

----------
System.Exception: TODO: Circular dependency references not supported. Package 'System.Net.Http'.
   at Microsoft.Framework.PackageManager.RestoreOperations.<>c__DisplayClass0.<ChainPredicate>b__2(String name)
   at Microsoft.Framework.PackageManager.RestoreOperations.<CreateGraphNode>d__1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task)
   at Microsoft.Framework.PackageManager.RestoreOperations.<CreateGraphNode>d__1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task)
   at Microsoft.Framework.PackageManager.RestoreOperations.<CreateGraphNode>d__1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task)
   at Microsoft.Framework.PackageManager.RestoreCommand.<RestoreForProject>d__1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Framework.PackageManager.RestoreCommand.<ExecuteCommand>d__1.MoveNext()
----------

[davidfowl]

This happens in the runtime because we don't fail for circular references right now:

https://github.com/aspnet/KRuntime/blob/dev/src/Microsoft.Framework.Runtime/DependencyManagement/WalkContext.cs#L87

Cycles are broken here but this doesn't flow out of the dependency walker. We should probably just throw instead.

[muratg]

Is this actually considered a cycle though? Shouldn't we treat framework assembly dependencies differently than package/project dependencies?

System.Net.Http.nuspec

<?xml version="1.0"?>
<package xmlns="http://schemas.microsoft.com/packaging/2010/07/nuspec.xsd">
  <metadata>
    <version>2.0.20710.0</version>
    <authors>Microsoft</authors>
    <owners>Microsoft</owners>
    <licenseUrl>http://www.microsoft.com/web/webpi/eula/MVC_4_eula_ENU.htm</licenseUrl>
    <projectUrl>http://www.asp.net/web-api</projectUrl>
    <dependencies>
      <dependency id="Microsoft.Net.Http" version="[2.0.20710.0, 2.1)" />
    </dependencies>
    <id>System.Net.Http</id>
    <title>System.Net.Http</title>
    <requireLicenseAcceptance>true</requireLicenseAcceptance>
    <description>Legacy package, System.Net.Http is now included in the 'Microsoft.Net.Http' package.</description>
    <summary />
    <language>en-US</language>
  </metadata>
</package>

Microsoft.Net.Http.nuspec

<?xml version="1.0"?>
<package xmlns="http://schemas.microsoft.com/packaging/2010/07/nuspec.xsd">
  <metadata>
    <version>2.0.20710.0</version>
    <authors>Microsoft</authors>
    <owners>Microsoft</owners>
    <licenseUrl>http://www.microsoft.com/web/webpi/eula/MVC_4_eula_ENU.htm</licenseUrl>
    <projectUrl>http://www.asp.net/web-api</projectUrl>
    <frameworkAssemblies>
      <frameworkAssembly assemblyName="System.Net.Http" targetFramework=".NETFramework4.5" />
      <frameworkAssembly assemblyName="System.Net.Http.WebRequest" targetFramework=".NETFramework4.5" />
    </frameworkAssemblies>
    <id>Microsoft.Net.Http</id>
    <title>Microsoft .NET Framework 4 HTTP Client Libraries</title>
    <requireLicenseAcceptance>true</requireLicenseAcceptance>
    <description>This package provides a programming interface for modern HTTP applications. This package includes HttpClient for sending requests over HTTP, as well as HttpRequestMessage and HttpResponseMessage for processing HTTP messages.</description>
    <language>en-US</language>
  </metadata>
</package>

[davidfowl]

Nope, if it stack overflows it's a cycle. The only difference between the two is fallback semantics.

[muratg]

So are you saying that this package is bad and it should fail? The same package seems to work with a traditional project type and nuget though.

[muratg]

@davidfowl is this good to close?

[davidfowl]

Closing this and opening a new bug about framework references being treated as packages