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

[mono] Disable unsupported apply update scenarios #55698

Merged
merged 5 commits into from
Jul 15, 2021

Conversation

lambdageek
Copy link
Member

  • Disallow mixing managed ApplyUpdate and debugger protocol apply update
  • Disallow connecting a debugger after ApplyUpdate has been called
  • Disallow calling ApplyUpdate if debugger is attached

Note that I didn't do the last 2 changes on wasm, since the debugging there doesn't fully work yet and it would impede further work to disable stuff now.

Fixes #55228, contributes to #44806

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-EnC-mono Hot Reload for WebAssembly, iOS/Android, etc label Jul 15, 2021
@lambdageek
Copy link
Member Author

@thaystg Do the debugger changes make sense?

@stephentoub I think this matches the scenarios we discussed.

@mikem8361 I think this is consistent with what CoreCLR does?

@stephentoub
Copy link
Member

I think this matches the scenarios we discussed.

The second two bullets. I'm not sure what the first bullet means?

@lambdageek lambdageek mentioned this pull request Jul 15, 2021
51 tasks
@lambdageek
Copy link
Member Author

lambdageek commented Jul 15, 2021

I think this matches the scenarios we discussed.

The second two bullets. I'm not sure what the first bullet means?

I think the first bullet is more of a technical point (in case I messed up the first two). In the core hot reload method, I keep track of whether we are called due to a debugger command or due to a call to the managed ApplyUpdate API and throw an exception if we see a mix of the two. The other two changes should prevent that from ever happening, but it seemed easy and harmless to add an extra check.

@stephentoub
Copy link
Member

Got it. Sounds right then. Thanks.

@lambdageek lambdageek merged commit 2e93fab into dotnet:main Jul 15, 2021
@mikem8361
Copy link
Member

I know it is late, but your logic looks good and follows coreclr's.

@ghost ghost locked as resolved and limited conversation to collaborators Aug 14, 2021
@lambdageek lambdageek deleted the fix-gh-55228 branch March 19, 2022 16:45
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[mono] block debugger attach after managed ApplyUpdate is called
4 participants