-
Notifications
You must be signed in to change notification settings - Fork 447
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
[release/5.0.1xx-preview3] Update dependencies from dotnet/runtime #7005
[release/5.0.1xx-preview3] Update dependencies from dotnet/runtime #7005
Conversation
…0331.15 - Microsoft.NETCore.Platforms - 5.0.0-preview.3.20181.15 - Microsoft.NETCore.DotNetHostResolver - 5.0.0-preview.3.20181.15 - Microsoft.NETCore.App.Host.win-x64 - 5.0.0-preview.3.20181.15 - Microsoft.NETCore.App.Internal - 5.0.0-preview.3.20181.15 - Microsoft.NETCore.App.Ref - 5.0.0-preview.3.20181.15 - Microsoft.NETCore.App.Runtime.win-x64 - 5.0.0-preview.3.20181.15
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
@dsplaisted @wli3 This looks to be the same issue as #6876 |
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
…0401.7 - Microsoft.NETCore.Platforms - 5.0.0-preview.3.20201.7 - Microsoft.NETCore.DotNetHostResolver - 5.0.0-preview.3.20201.7 - Microsoft.NETCore.App.Host.win-x64 - 5.0.0-preview.3.20201.7 - Microsoft.NETCore.App.Internal - 5.0.0-preview.3.20201.7 - Microsoft.NETCore.App.Ref - 5.0.0-preview.3.20201.7 - Microsoft.NETCore.App.Runtime.win-x64 - 5.0.0-preview.3.20201.7
…0402.4 - Microsoft.NETCore.Platforms - 5.0.0-preview.3.20202.4 - Microsoft.NETCore.DotNetHostResolver - 5.0.0-preview.3.20202.4 - Microsoft.NETCore.App.Host.win-x64 - 5.0.0-preview.3.20202.4 - Microsoft.NETCore.App.Internal - 5.0.0-preview.3.20202.4 - Microsoft.NETCore.App.Ref - 5.0.0-preview.3.20202.4 - Microsoft.NETCore.App.Runtime.win-x64 - 5.0.0-preview.3.20202.4
@jeffschwMSFT @janvorli Can you take a look at this? Your fix has made it to the PR but test processes still appear to be crashing |
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
Sure, I'll take a look |
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
It seems that it was running dotnet from the preview 3 at the time of the crash. Is the fix already in preview 3 too? The paths in the crash stack trace contain
|
@janvorli Yep, that's this commit. I ported it over: dotnet/runtime@2b487f3 |
Also still seen in master: https://github.com/dotnet/installer/pull/7018/checks?check_run_id=557580302 |
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
Ok, I'm going to try it locally. |
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
Ah, so this is actually a different issue than the one I've fixed. Valgrind reports:
I need to get debug symbols with line numbers to make more sense on where it happens. It looks like we allocate something and then store a pointer in the middle of that into a hash table which later tries to free that memory. |
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
/fyi @sfoslund Separate bug. |
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
I have tried to figure out what's wrong and first fetched symbols for the libhostpolicy.so and libhostfxr.so. Then Valgrind has reported line numbers too. The 520 bytes allocated block it refers to is the fx_definition_t instance created at https://github.com/dotnet/runtime/blob/dad74cc683277f7919da79871626359d6aa812b1/src/installer/corehost/cli/hostpolicy/hostpolicy_init.cpp#L88 and the address that is passed to the free is at offset 496 in this data type. When running under lldb, I have found that there is the following field at that offset: The place in libhostpolicy.so where the call chain to the freeing of the wrong memory stems from is https://github.com/dotnet/runtime/blob/dad74cc683277f7919da79871626359d6aa812b1/src/installer/corehost/cli/deps_format.cpp#L376 It seems the problem can be caused by the fact that we are mixing dotnet host app from .NET Core 2.1 with libhostpolicy.so / libhostfxr.so from 5.0 when both has some standard C++ libraries code for hash table compiled in. As you can see we basically work with the same hash map from both libhostpolicy.so and dotnet. Each was compiled with a different clang version and maybe even different standard c++ library version. So I wouldn't be surprised if there were some compatibility issues. Is the .dotnet/dotnet intentionally the version from .NET Core 2.1? |
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
That seems wrong... looping in @wli3 and @dsplaisted |
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
We install the .NET Core 2.1 runtime here: https://github.com/dotnet/installer/blob/master/eng/restore-toolset.sh#L19 I would guess that this is overwriting @sfoslund Do you want to try this? |
|
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
But i don't think we are mixing anything. That script just drop 2.1 to dotnet folder, which is common user facing scenario -- having 5.0.100 sdk with 2.1 runtime |
From @eerhardt in dotnet/sdk#11145 |
@mmitche per fix install-script. I don't think it is an easy fix. Actually I can only find a hacky solution. More in dotnet/sdk#11145 |
@janvorli Can you run with |
It didn't help |
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
A little more info on what I wrote above: The original design of the host was for the
And, lastly, |
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
@lpereira This is from the LD_DEBUG log: |
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
Btw, here are the end to end repro steps on Ubuntu 16.04 - I've removed the --docker option from the build command used in the CI so that I can easily debug it. (please replace the /home/janvorli/issues/installer with whatever directory you are using, I am not sure if all the absolute paths are needed, but I didn't want to spend time experimenting): cd /home/janvorli/issues/installer
git fetch --force --tags --prune --progress --no-recurse-submodules origin +refs/heads/*:refs/remotes/origin/* +refs/pull/7018/merge:refs/remotes/pull/7018/merge
git checkout --progress --force refs/remotes/pull/7018/merge
./build.sh --test --pack --publish --ci --noprettyprint --configuration Debug --architecture x64 You can break the build once you can see the crashes Finally, this reproduces the crash: /home/janvorli/issues/installer/.dotnet/dotnet /home/janvorli/issues/installer/.nuget/packages/testsdkclean/5.0.100-preview.3.20174.1/tools/netcoreapp5.0/any/Microsoft.NET.Clean.Tests.dll -noautoreporters -noRepoInference -dotnetPath /home/janvorli/issues/installer/artifacts/bin/redist/Debug/dotnet-with-previous-runtimes/dotnet -testExecutionDirectory /home/janvorli/issues/installer/artifacts/tmp/Debug/dotnetSdkTests -testConfigFile /home/janvorli/issues/installer/test/SdkTests/TestConfig.xml -xml /home/janvorli/issues/installer/artifacts/TestResults/Debug/Microsoft.NET.Clean.Tests.xml -html /home/janvorli/issues/installer/artifacts/TestResults/Debug/Microsoft.NET.Clean.Tests.html -testList SdkIntegrationTests |
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
dotnet/runtime#34534 fixed insertions into master and has been ported to preview 3 with dotnet/runtime#34570 but it looks like we haven't been able to produce a build of the runtime since thursday so the change isn't flowing. |
Auto-Merge StatusThis pull request has not been merged because Maestro++ is waiting on the following merge policies.
|
Currently building now...failed but will investigate. May just be AzDO flakiness |
Auto-Merge StatusThis pull request has been merged because the following merge policies have succeeded.
|
…0406.14 - Microsoft.NETCore.Platforms - 5.0.0-preview.3.20206.14 - Microsoft.NETCore.DotNetHostResolver - 5.0.0-preview.3.20206.14 - Microsoft.NETCore.App.Host.win-x64 - 5.0.0-preview.3.20206.14 - Microsoft.NETCore.App.Internal - 5.0.0-preview.3.20206.14 - Microsoft.NETCore.App.Ref - 5.0.0-preview.3.20206.14 - Microsoft.NETCore.App.Runtime.win-x64 - 5.0.0-preview.3.20206.14
This pull request updates the following dependencies
From https://github.com/dotnet/runtime