Skip to content

Fix GetAppdomainStaticAddress for non-collectible assemblies#41076

Merged
davmason merged 4 commits into
dotnet:masterfrom
davmason:getappdomainstaticaddress
Aug 24, 2020
Merged

Fix GetAppdomainStaticAddress for non-collectible assemblies#41076
davmason merged 4 commits into
dotnet:masterfrom
davmason:getappdomainstaticaddress

Conversation

@davmason
Copy link
Copy Markdown
Contributor

@davmason davmason commented Aug 20, 2020

Fixes #40954

#34677 had a bug in it that it would check if pModule->GetLoaderAllocator()->GetExposedObject was NULL, which is the right thing to check for collectible modules but non-collectible modules it will always be NULL.

@davmason davmason added this to the 5.0.0 milestone Aug 20, 2020
@davmason davmason requested a review from a team August 20, 2020 07:06
@davmason davmason self-assigned this Aug 20, 2020
@ghost
Copy link
Copy Markdown

ghost commented Aug 20, 2020

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Comment thread src/coreclr/src/vm/proftoeeinterfaceimpl.cpp
Copy link
Copy Markdown
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@davmason
Copy link
Copy Markdown
Contributor Author

@hoyosjs there are a couple failures with "cmd is not recognized as an internal or external command" like https://dev.azure.com/dnceng/public/_build/results?buildId=781079&view=ms.vss-test-web.build-test-results-tab&runId=24584748&resultId=101778&paneView=debug

Is this a known issue?

@hoyosjs
Copy link
Copy Markdown
Member

hoyosjs commented Aug 21, 2020

Is this a known issue?

Yes, this is https://github.com/dotnet/core-eng/issues/10586

@davmason davmason merged commit c6b4af3 into dotnet:master Aug 24, 2020
@davmason
Copy link
Copy Markdown
Contributor Author

/backport to release/5.0

@github-actions
Copy link
Copy Markdown
Contributor

Started backporting to release/5.0: https://github.com/dotnet/runtime/actions/runs/222712648

@sandreenko
Copy link
Copy Markdown
Contributor

There was a test failure in a recent outerloop, is it a known issue?
https://dev.azure.com/dnceng/public/_build/results?buildId=794520&view=ms.vss-test-web.build-test-results-tab

  Starting:    profiler.unittest.XUnitWrapper
    profiler\unittest\getappdomainstaticaddress\getappdomainstaticaddress.cmd [FAIL]
      Unhandled exception. System.Exception: Profiler tests are expected to contain the text 'PROFILER TEST PASSES' in the console output of the profilee app to indicate a passing test. Usually it is printed from the Shutdown() method of the profiler implementation. This text was not found in the output above.
         at Profiler.Tests.ProfilerTestRunner.LogTestFailure(String error) in /__w/1/s/src/tests/profiler/common/ProfilerTestRunner.cs:line 138
         at Profiler.Tests.ProfilerTestRunner.Run(String profileePath, String testName, Guid profilerClsid, String profileeArguments, ProfileeOptions profileeOptions) in /__w/1/s/src/tests/profiler/common/ProfilerTestRunner.cs:line 92
         at Profiler.Tests.Program.Main(String[] args) in /__w/1/s/src/tests/profiler/unittest/getappdomainstaticaddress.cs:line 25

@davmason
Copy link
Copy Markdown
Contributor Author

davmason commented Sep 4, 2020

@sandreenko no that is not a known issue. Looking at the logs it's not a failure I would expect to see either. I'll try and dig a little deeper but I suspect that this is a one off thing I won't be able to investigate

@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
@davmason davmason deleted the getappdomainstaticaddress branch January 20, 2021 08:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GetAppDomainStaticAddress only works for collectible modules

5 participants