[release/9.0-staging] [mono][interp] Fix various leaks, primarily around dynamic methods #120600
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…otnet#119176) * [mono][interp] Fix leaking of compilation data For dynamic methods, imethod_alloc0 allocates from the dynamic method's mempool which is freed when the method is collected. We were previously allocating only from the global memory manager. * [mono][interp] Stop leaking data items Previously we were registering imethod locations from these data items in order for them to be patched once a method is tiered up. Because of this registering, the data item had to always be around. We now free the data item for dynamic methods and also we deregister the patch locations when freeing such a method. * [mono][interp] Free headers allocated for inlined methods We add them to a list for later freeing. This uses the same pattern as jit. * [mono][interp] Skip interp free for methods not yet compiled
…ed (dotnet#119749) * [mono] Actually free dynamic methods, even if we have profiler attached Ever since the early days of mono, freeing dynamic methods was disabled if a profiler was attached. The reason for this was probably that the profiler might store `MonoMethod` instances in its own data, leading to problems if we free those instances. Looking at the profilers nowadays it is not clear where patterns like this would happen. Profilers that do store methods (like aot, coverage), don't process wrapper methods, so we should be safe since dynamic methods have the MONO_WRAPPER_DYNAMIC_METHOD wrapper type. The problem is that, nowadays, we can always have a profiler attached even if no actual profiling happens. macios for example always calls mono_profiler_install. I belive actual callbacks can be added later as necessary. * Disable freeing dynamic methods only when eventpipe or debugger are enabled
…et#119990) The code of a compiled method can use data items that contain InterpMethod* pointers. Because at the moment when a method is compiled these referenced interp methods weren't yet tiered up, we will register these data item locations to the tiering backend so that, when they do get tiered up, the locations get updated with the new InterpMethod* reference. At this moment of compilation time, we could race with other compilers of the same InterpMethod so we could end up registering redundant data_items (only the data_items for the method winning the race will actually end up being used by the executing interpreter code). Normally this is harmless, we just patch some data that never gets used. The problems start when we free interpreter methods. When the interp method is freed, we need to clear the patch_sites_table of any locations pointing into this method's data items. We do a search starting from the data items of this method. Because we have no way to reference the wrongly registered data items, these entries will stay alive in the table. As the memory gets reused, these entries will point into other runtime code, and when the get patched, it will result in corruption of memory. We fix this issue by registering the patch sites only for the method winning the compilation race (the one that first sets the transformed flag).
If the MONO_ENABLE_DYNMETHOD_FREE is not passed, the behavior is not changed at all in order to mitigate potential risk. The old behavior was that if we have a profiler installed we don't free dynamic methods, which was always the case on maui.
There was a problem hiding this comment.
Pull Request Overview
This PR backports multiple changes from main to fix memory leaks in the Mono interpreter, particularly around dynamic methods. The fixes address customer-reported memory leaks where repeated workflow execution on maui-ios resulted in approximately 60MB of leaked memory.
Key changes include:
- Adding proper cleanup of method headers and data items during interpreter method destruction
- Implementing patch site clearing for tiering functionality to prevent dangling references
- Adding conditional dynamic method freeing controlled by environment variable for reduced risk
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/mono/mono/mini/interp/transform.h | Adds headers_to_free field to track method headers requiring cleanup |
| src/mono/mono/mini/interp/transform.c | Implements header tracking, changes memory allocation strategy, and adds proper cleanup |
| src/mono/mono/mini/interp/tiering.h | Declares new function for clearing data item patch sites |
| src/mono/mono/mini/interp/tiering.c | Implements patch site clearing logic to remove dangling references |
| src/mono/mono/mini/interp/interp.c | Adds call to clear patch sites during method freeing |
| src/mono/mono/mini/interp/interp-internals.h | Adds n_data_items field to track data item count |
| src/mono/mono/metadata/loader.c | Adds environment variable control for dynamic method freeing |
BrzVlad
added
Servicing-consider
kotlarmilos
approved these changes
Oct 10, 2025
BrzVlad
added
Servicing-approved
|
Reminder that PRs into release/9.0-staging need to be completed by 4pm Redmond time on October 13 to make it into the November servicing release. After that point you'll need to target release/9.0 if you want to make the November release. |
|
/ba-g wasm failure unrelated |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This backports multiple changes that were added on main in order to fix various leaks around interpreter and dynamic methods.
#119176
#119294
#119749
#119990
Customer Impact
On maui-ios, dynamic method execution is done with the interpreter. Freeing of dynamic methods was disabled for a long time and a few leaks were present in the interpreter, preventing completely freeing all the data associated with an interpreter method. Interpreter is more heavily used on maui, compared to Xamarin, so this is more likely to become a problem. On a customer application, starting and closing a workflow 14 times resulted in leaking of around 60MB of memory.
Regression
Testing
Tested on a heavy customer application that it works correctly. This path is also commonly hit as part of our libraries tests and all these changes have been in main for multiple weeks now, without any issues. This was also validated by the customer on their app, by having the fix deployed for early testing.
Risk
I would say between low and moderate. Most of this change is quite trivial and it just adds freeing of some data or changing the memory allocator for certain interpreter compilation data, so that it gets freed. There is a more complex part of the change, that adds freeing for data used by tiering. While this part would have a higher risk, it is isolated to the tiering machinery, and customers have an easy way to disable tiering, effectively mitigating the risk in my opinion. Compared to the .net10 backport, there is one more commit in this PR which reverts part of the change by default, having it enabled only when an env var is passed, to further reduce the risk.