Implement Startup hooks support in Mono

[lambdageek]

Fixes #47462

CoreCLR This also makes some changes to CoreCLR to decouple EventPipe and startup hooks. Presently if startup hooks are disabled, RuntimeEventSource.Initialize is never called. The PR makes the two features independent by moving runtime event source initialization out of the startup hook feature check.
TODO:

• Make ios functional test
• Make android functional test
• Make wasm functional test
• Make an Issue to add mobile and wasm workload samples (once we have WBT for mobile) ==> [test] Add mobile workload test for startup hooks for Mono #80742
• Fix error handling - the startup hooks design doc says the app should terminate if a hook has an unhandled exception. Right now mono cleans up the error and ignores it.

[vargaz]

The mono changes look ok to me.
Would this have any impact on linked app size and startup time ?

[lambdageek]

The mono changes look ok to me. Would this have any impact on linked app size and startup time ?

Linked size shouldn't be affected. Unless you set <StartupHookSupport>true</StartupHookSupport> the .NET SDK will set IsSupported to false, and the managed code should drop out.

Startup time might be affected a little bit because of the StartupHookProvider lookup.

[vitek-karas]

The CoreLib changes look good.

[vitek-karas]

Curious about the trimming story - in CoreCLR we simply don't support startup hooks with trimming. But this seems to make some effort to do that in mono. The problem is that startup hooks are effectively plugins, and so with trimmed framework the plugins may break.

[lambdageek]

Curious about the trimming story - in CoreCLR we simply don't support startup hooks with trimming. But this seems to make some effort to do that in mono. The problem is that startup hooks are effectively plugins, and so with trimmed framework the plugins may break.

The primary use case for mobile right now is to allow hot reload to inject an agent into the app. Injecting the hook is actually a pretty static operation - the agent assembly has to make it into the app bundle that gets uploaded to the device or emulator - so there is a PackageReference, the StartupHookSupport SDK property is toggled (which toggles the feature flag), etc. So everything should be visible to the trimmer if it even runs (in Debug configurations the trimmer won't be trimming anything).

The only other trimming that is happening is the initial trimming of System.Private.CoreLib when we create the runtime packs. At that stage we say that runtime hook switch defaults to true and we add the process hooks function to the linker descriptor so it's kept around. It's possible that we drop some private CoreLib APIs that a startup hook may want to use that are otherwise completely unused in the runtime and aren't in the linker descriptor. That's always a risk with private reflection.

---

In general, I think any sort of more dynamic startup hook injection is unlikely to work on mobile. There's really no good way to get code onto a device without it being part of the app bundle. But running code before the user (or platform) code starts running is still useful.

[lambdageek]

The event tracing failures on osx-x64 repro locally. Not sure how they're related but I'm sure it'll be educational...

[lambdageek]

@lateralusX @dotnet/dotnet-diag

So believe it or not the failure I'm seeing is due to the call in StartupHookProvider to RuntimeEventSource.Initialize(). Turns out mono already invokes that method from ep_finish_init. So when we run it a second time, we instantiate a second RuntimeEventSource instance and then things go wrong.

Questions:

1. As far as I can tell, coreclr only calls RuntimeEventSource.Initialize from StartupHookProvider. Which means if !StartupHookProvider.IsSupported, the runtime event source is not initialized? Is that intentional? I could make mono do the same thing, but it seems weird to couple these two features. Or is there a second call site in coreclr somewhere?
2. Alternately, I could make RuntimeEventSource.Initialize safe to call multiple times by only creating a new instance if there is not one already (probably using a lock-object variant of LazyInitializer.EnsureInitialized)
3. Alternately, I could add #if !MONO to StartupHookProvider and leave both runtimes with the same behavior we have today

Update I added option 2 to this PR, but we can go with another approach
Update 2 actually trying another option:
Make ep_rt_init_finish for CoreCLR invoke RuntimeEventSource.Initialize same as in mono, and remove the init call from StartupHookProvider

[noahfalk]

As far as I can tell, coreclr only calls RuntimeEventSource.Initialize from StartupHookProvider. Which means if !StartupHookProvider.IsSupported, the runtime event source is not initialized? Is that intentional?

No, that doesn't sound like a desired behavior to me. I don't see any reason that RuntimeEventSource should be conditional on having startup hooks enabled. I'm guessing what happened is that someone noticed it was important to initialize RuntimeEventSource prior to running startup hooks, and then separately someone else added an option to disable startup hooks and didn't recognize that the RuntimeEventSource.Initialize() should not be included in the scope of code that gets disabled. I'd propose we hoist the call to RuntimeEventSource.Initialize() outside the ProcessStartupHooks call.

[lambdageek]

I'd propose we hoist the call to RuntimeEventSource.Initialize() outside the ProcessStartupHooks call.

So change this to something more generic and call both the RuntimeEventSource initialization and the startup hook initialization from there?

runtime/src/coreclr/vm/assembly.cpp

Lines 1424 to 1437 in 210a7a9

	static void RunStartupHooks()
	{
	CONTRACTL
	{
	THROWS;
	GC_TRIGGERS;
	MODE_COOPERATIVE;
	INJECT_FAULT(COMPlusThrowOM(););
	}
	CONTRACTL_END;
	MethodDescCallSite processStartupHooks(METHOD__STARTUP_HOOK_PROVIDER__PROCESS_STARTUP_HOOKS);
	processStartupHooks.Call(NULL);
	}

[noahfalk]

So change this to something more generic and call both the RuntimeEventSource initialization and the startup hook initialization from there?

Yeah, that sounds good to me. Perhaps something like:

void ManagedStartup() // call this from native runtime startup code
{
    InitBeforeUserCode(); // run EventSource initialize in here
    ProcessStartupHooks(); // do the rest of the pre-existing startup hook logic here
}

[lambdageek]

So change this to something more generic and call both the RuntimeEventSource initialization and the startup hook initialization from there?

Yeah, that sounds good to me. Perhaps something like:

void ManagedStartup() // call this from native runtime startup code
{
    InitBeforeUserCode(); // run EventSource initialize in here
    ProcessStartupHooks(); // do the rest of the pre-existing startup hook logic here
}

Hmm... that's actually not very linker friendly. Ideally I'd like to trim ManagedStartup if its body is entirely empty because both eventpipe and startup hooks are disabled. Otherwise this could be a startup perf regression for Mono in published mobile/wasm apps.

I guess the other way to go here is to make coreclr more like mono: call the RuntimeEventSource.Initialize from ep_rt_init_finish (ultimately called by EnsureEEStarted from CorHost2::Start()) and remove it from StartupHookProvider.ProcessStartupHooks (called from ExecuteMainMethod from Corhost2::ExecuteAssembly).

[lambdageek]

Okay, CoreCLR now follows Mono's model: RuntimeEventSource.Initialize is invoked from EventPipeAdapter::FinishInitialization() and is not related to startup hooks.

[lateralusX]

Mono bits LGTM!

[lambdageek]

Verified manually (by overwriting files in runtime packs) that the Microsoft.Android SDK works with this implementation of startup hooks

[lambdageek]

CoreCLR eventsourceerror failure is #80666

I think this is ready to merge

[lambdageek]

linux-arm64 llvmfullaot timeout is known #80805