castleproject / Core

Castle Core, including Castle DynamicProxy, Logging Services and DictionaryAdapter
http://www.castleproject.org/
Other
2.2k stars 467 forks source link

When Castle.Core is loaded in a collectible Assembly Load Context, use AssemblyBuilderAccess.RunAndCollect by default #679

Open simonferquel opened 2 months ago

simonferquel commented 2 months ago

fix #473

The context of this work is about working on testing support in a future version of Unity running on CoreCLR instead of Mono. In that version, all user code (including tests) and their dependencies (except for Unity own assemblies) are loaded in a collectible Assembly Load Context, in order to support code changes without reloading Unity Editor - we have our own test runner as part of Unity Editor. In that context, any test written with mocking frameworks relying on Castle.Core such as Moq will leak the user code ALC on whenever the editor decides to reload code. This PR changes the default AssemblyBuilderAccess used under CoreCLR when Castle.Core itself is loaded in a collectible ALC, so that anything it generates get collectible as well.

stakx commented 1 month ago

Thanks for the PR. On the surface, this seems like a reasonable suggestion though I'll need to think more carefully about whether it'll work fine by default in all cases. Mainl this: is the Castle.Core always guaranteed to be referenced only by collectible assemblies if it is loaded in a collectible context?

Also, tied to the above question, I wonder if downstream code actually needa to be able to ever override the default setting being introduced here. If not then it might be preferrable not to expose a new public type.

I'll add a few minor code review comments and hopefully get some time to investigate the first point above.

simonferquel commented 1 month ago

About making the type public and overridable, there is already someone who commented about needing it here: https://github.com/castleproject/Core/issues/473#issuecomment-2235949260