Skip to content
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/7.0] [hot_reload] Fix unresolved token when new nested structs are used #76625

Merged
merged 2 commits into from
Oct 5, 2022

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Oct 4, 2022

Backport of #76618 to release/7.0

/cc @lambdageek

Customer Impact

Customers using .NET 7 hot reload on iOS/Android/WebAssembly may experience application crashes if using the new .NET 7 capability to add new class definitions during a hot reload session.

In .NET 7 for iOS/Android/WebAssembly it is now possible to add new methods (to existing classes) as well as newly defined classes/structs/enums during a hot reload session. However in some circumstances (when a new nested struct definition is used as the type of a field in a new class) adding such new definitions causes the runtime to crash.

Testing

New CI test; manual testing.

Risk

Low or Moderate. The fix involves causing the runtime loader to delay initializing the MonoClass that represents the nested struct. It is possible that not every code path that attempts to use the new field actually does this initialization. I have attempted to verify that class fields are used by the mono interpreter only after explicitly initializing them, or through accessor methods that explicitly initialize them. However it is possible that fixing this regression may uncover other underlying issues.

On the other hand in all such cases it should be possible to simply restart the app. It's not the ideal developer inner loop, but it would not leave the developer completely blocked.

None of the changes here impact apps deployed in production.

This fails with

```
       Stack Trace:

          Child exception:
            System.BadImageFormatException: Could not resolve field token 0x04000014
          File name: 'System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType'
          /Users/alklig/work/dotnet-runtime/runtime/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType.cs(29,0):
          at
          System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType.ZExistingClass.ExistingMethod()

```
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-EnC-mono Hot Reload for WebAssembly, iOS/Android, etc label Oct 4, 2022
@lambdageek lambdageek added this to the 7.0.0 milestone Oct 4, 2022
@lambdageek lambdageek added the Servicing-consider Issue for next servicing release review label Oct 4, 2022
@carlossanlop
Copy link
Member

@lambdageek if you haven't done so, can you please send the email to Tactics requesting approval?
@vargaz @thaystg can you please provide a code review sign-off?

@lambdageek lambdageek added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Oct 5, 2022
@lambdageek
Copy link
Member

Approved by tactics in email

@carlossanlop
Copy link
Member

CI failure is #76680

Approved by Tactics, signed off. Ready to merge. :shipit:

@carlossanlop carlossanlop merged commit 44734a5 into release/7.0 Oct 5, 2022
@carlossanlop carlossanlop deleted the backport/pr-76618-to-release/7.0 branch October 5, 2022 18:06
@ghost ghost locked as resolved and limited conversation to collaborators Nov 4, 2022
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.

3 participants