Skip to content

Conversation

thaystg
Copy link
Member

@thaystg thaystg commented Apr 1, 2025

Backport of #113995 to release/9.0-staging

/cc @thaystg

Customer Impact

  • Customer reported
  • Found internally

https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2396448
When a customer is trying to apply hotreload on mono runtime depending on how many things are added the hotreload will not work and may hit an assertion or have any weird behavior about not finding methods/assembly with names containing trash value.

Any update in an app that any metadata table is near the size of 65536 will cause it in the first change.
Or any huge change that causes the metadata table size increase.

Regression

  • Yes
  • No

[If yes, specify when the regression was introduced. Provide the PR or commit if known.]

Testing

Manually tested and also created a test case.

Risk

[High/Medium/Low. Justify the indication by mentioning how risks were measured and addressed.] Medium risk, because we changed how we read/save the metadata information after a hot reload. But this will only affect mono hotreload customers.

@Copilot Copilot AI review requested due to automatic review settings April 1, 2025 20:12
@thaystg thaystg requested a review from marek-safar as a code owner April 1, 2025 20:12
@ghost ghost added the area-EnC-mono Hot Reload for WebAssembly, iOS/Android, etc label Apr 1, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a new test case to verify that metadata row sizes are adjusted correctly and introduces an autogenerated helper class for the update. Key changes include:

  • A new test, TestIncreaseMetadataRowSize, in ApplyUpdateTest.cs validating metadata updates.
  • Addition of the autogenerated IncreaseMetadataRowSize class in a new file.

Reviewed Changes

Copilot reviewed 3 out of 7 changed files in this pull request and generated 2 comments.

File Description
src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs Added a new test case checking metadata row size and method signature.
src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.IncreaseMetadataRowSize/IncreaseMetadataRowSize.cs Autogenerated class with placeholder methods for metadata updates.
Files not reviewed (4)
  • src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.IncreaseMetadataRowSize/System.Reflection.Metadata.ApplyUpdate.Test.IncreaseMetadataRowSize.csproj: Language not supported
  • src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.IncreaseMetadataRowSize/deltascript.json: Language not supported
  • src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj: Language not supported
  • src/mono/mono/component/hot_reload.c: Language not supported

@carlossanlop
Copy link
Contributor

Friendly reminder that code complete is on April 14th for the May Release. If you'd like to get this change included in that release, please get a Tactics approval and merge this PR before that date.

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.

lgtm. we will take for consideration in 9.0.x

@jeffschwMSFT jeffschwMSFT added the Servicing-consider Issue for next servicing release review label Apr 7, 2025
@jeffschwMSFT jeffschwMSFT added this to the 9.0.x milestone Apr 7, 2025
@leecow leecow added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Apr 8, 2025
@leecow leecow modified the milestones: 9.0.x, 9.0.5 Apr 8, 2025
@carlossanlop
Copy link
Contributor

Friendly reminder that code complete for the May release is next Monday April 14th. If you want this change included in that release, please merge the PR before EOD Monday.

@thaystg
Copy link
Member Author

thaystg commented Apr 11, 2025

/ba-g CI unrelated failures

@thaystg thaystg merged commit 5c8176d into dotnet:release/9.0-staging Apr 11, 2025
115 of 123 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators May 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-EnC-mono Hot Reload for WebAssembly, iOS/Android, etc Servicing-approved Approved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants