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

[simonferquel]

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]

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.

[stakx]

Wrong year, should be -2024.

[stakx]

Note to self: need to research what this conditional compilation symbol is for, and whether it's needed.

Does it guard the RunAndCollect enum value, or the AssemblyLoadContext.GetLoadContext? If the former, then perhapa the whole type being added here shouldn't be exposed publicly for those TFMs; so I'm suspecting it's probably guarding the latter.

[simonferquel]

It is to guard against usage of AssemblyLoadContext (it is not in netatandard / netfx, only in .net core).

However, it would be nice if the check could be done at runtime (in case of loading netatandard assemblies in a .net core executable). We have usage of reflection based wrappers at Unity for this kind of thing. I can probably do the same here.

[simonferquel]

About making the type public and overridable, there is already someone who commented about needing it here: #473 (comment)

[stakx]

@simonferquel, I am going to close this in favor of the alternative #685, which is a closer fit with DynamicProxy's pre-existing APIs. Thanks nevertheless for contributing!

[simonferquel]

That is not ideal, as CastleCore is often used trough higher level libraries such as Moqing frameworks. That means those frameworks will have to themselves expose the feature to avoid breaking Unity...

[stakx]

Why is that a problem? So let those libraries implement the necessary logic. DynamicProxy is a generic, fairly low-level tool, I think it makes sense not to apply too many default behaviours in anticipation of one specific use case.

Or, in different terms: can you give a guarantee to all DynamicProxy users that the auto-detection logic will always do the right thing? If yes, then we can consider putting it in DynamicProxy, but we need to identify all relevant test scenarios & add tests for them. If no, or we're not absolutely certain that the auto-detection logic will always decide correctly, then we better leave it to downstream code to decide which behavior they want.