Skip to content

Commit

Permalink
Don't shut down event pipe in DLLs on Windows (#91715)
Browse files Browse the repository at this point in the history
Works around #89346.

We blocked event pipe completely in shared libraries in #90811 but:

* Elinor found out the shutdown problem is actually Windows specific, so blocking Linux (our most important target) is unnecessary.
* We can avoid the hang at the cost of corrupting the ongoing event pipe session by just not doing the shutdown. This can be worked around by simply stopping the event pipe session at dotnet-trace side manually.

I validated the shared library scenario no longer hangs at shutdown with this.

#89346 still tracks if we can do better here.
  • Loading branch information
MichalStrehovsky authored Sep 14, 2023
1 parent 871f1b6 commit 9101b85
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 7 deletions.
10 changes: 8 additions & 2 deletions src/coreclr/nativeaot/Bootstrap/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ static char& __unbox_z = __stop___unbox;

#endif // _MSC_VER

extern "C" bool RhInitialize();
extern "C" bool RhInitialize(bool isDll);
extern "C" void RhSetRuntimeInitializationCallback(int (*fPtr)());

extern "C" bool RhRegisterOSModule(void * pModule,
Expand Down Expand Up @@ -164,7 +164,13 @@ extern "C" void __managed__Startup();

static int InitializeRuntime()
{
if (!RhInitialize())
if (!RhInitialize(
#ifdef NATIVEAOT_DLL
/* isDll */ true
#else
/* isDll */ false
#endif
))
return -1;

void * osModule = PalGetModuleHandleFromPointer((void*)&NATIVEAOT_ENTRYPOINT);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,7 @@
Text="RuntimeIdentifier is required for native compilation. Try running dotnet publish with the -r option value specified." />
<Error Condition="'$(GeneratePackageOnBuild)' == 'true'" Text="GeneratePackageOnBuild is not supported for native compilation." />
<Error Condition="'$(OutputType)' != 'Library' and '$(NativeLib)' != '' and '$(CustomNativeMain)' != 'true'" Text="NativeLib requires OutputType=Library." />
<!-- See https://github.com/dotnet/runtime/issues/89346 for details -->
<Error Condition="'$(NativeLib)' != '' and '$(EventSourceSupport)' == 'true'" Text="EventSource is not supported when compiling to a native library. Set EventSourceSupport to false." />
<Warning Condition="'$(NativeLib)' != '' and '$(EventSourceSupport)' == 'true' and '$(_SuppressNativeLibEventSourceWarning)' != 'true'" Text="EventSource is not supported or recommended when compiling to a native library. Please go to https://github.com/dotnet/runtime/issues/91762 for more details." />

<Error Condition="'$(PublishTrimmed)' == 'false'" Text="PublishTrimmed is implied by native compilation and cannot be disabled." />

Expand Down
22 changes: 19 additions & 3 deletions src/coreclr/nativeaot/Runtime/startup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,10 @@ static void UninitDLL()
Thread* g_threadPerformingShutdown = NULL;
#endif

#if defined(_WIN32) && defined(FEATURE_PERFTRACING)
bool g_safeToShutdownTracing;
#endif

static void __cdecl OnProcessExit()
{
#ifdef _WIN32
Expand All @@ -304,8 +308,16 @@ static void __cdecl OnProcessExit()
#endif

#ifdef FEATURE_PERFTRACING
EventPipe_Shutdown();
DiagnosticServer_Shutdown();
#ifdef _WIN32
// We forgo shutting down event pipe if it wouldn't be safe and could lead to a hang.
// If there was an active trace session, the trace will likely be corrupted without
// orderly shutdown. See https://github.com/dotnet/runtime/issues/89346.
if (g_safeToShutdownTracing)
#endif
{
EventPipe_Shutdown();
DiagnosticServer_Shutdown();
}
#endif
}

Expand Down Expand Up @@ -343,7 +355,7 @@ void RuntimeThreadShutdown(void* thread)
#endif
}

extern "C" bool RhInitialize()
extern "C" bool RhInitialize(bool isDll)
{
if (!PalInit())
return false;
Expand All @@ -352,6 +364,10 @@ extern "C" bool RhInitialize()
atexit(&OnProcessExit);
#endif

#if defined(_WIN32) && defined(FEATURE_PERFTRACING)
g_safeToShutdownTracing = !isDll;
#endif

if (!InitDLL(PalGetModuleHandleFromPointer((void*)&RhInitialize)))
return false;

Expand Down

0 comments on commit 9101b85

Please sign in to comment.