Skip to content

Conversation

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Aug 30, 2023

Backport of #91283 to release/8.0

/cc @janvorli

Customer Impact

When a customer code calls the AssemblyLoadContext.Unload method twice, runtime crashes with a FailFast. Only the first call to the Unload method should have any effect, further calls to it should be a no-op.

Testing

Directed test for the issue, libraries tests, coreclr tests, CI testing

Risk

Low, the change just ensures that the call to the underlying native runtime helper method is made only once.

The AssemblyLoadContext.InitiateUnload method was not handling multiple
calls correctly. Instead of making the extra calls dummy, it was
calling the native runtime helper again.

This change fixes it by simply ensuring that the runtime helper is
called just once.
@janvorli janvorli requested a review from jkotas August 30, 2023 17:26
@janvorli janvorli added this to the 8.0.0 milestone Aug 30, 2023
@janvorli janvorli self-assigned this Aug 30, 2023
@janvorli janvorli added Servicing-consider Issue for next servicing release review area-AssemblyLoader-coreclr labels Aug 30, 2023
@ghost
Copy link

ghost commented Aug 30, 2023

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #91283 to release/8.0

/cc @janvorli

Customer Impact

When a customer code calls the AssemblyLoadContext.Unload method twice, runtime crashes with a FailFast. Only the first call to the Unload method should have any effect, further calls to it should be a no-op.

Testing

Directed test for the issue, libraries tests, coreclr tests, CI testing

Risk

Low, the change just ensures that the call to the underlying native runtime helper method is made only once.

Author: github-actions[bot]
Assignees: janvorli
Labels:

Servicing-consider, area-AssemblyLoader-coreclr

Milestone: 8.0.0

@carlossanlop
Copy link
Contributor

@jkotas can you please give a code review sign-off?
@jeffschwMSFT do you approve the backport?

Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

approved. please get a code review. once ready this can be merged.

@jeffschwMSFT jeffschwMSFT added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Aug 30, 2023
@carlossanlop carlossanlop merged commit 7174092 into release/8.0 Aug 30, 2023
@carlossanlop carlossanlop deleted the backport/pr-91283-to-release/8.0 branch August 30, 2023 21:07
@radical radical mentioned this pull request Sep 26, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants