Skip to content

[NativeAOT] do not do shutdown for the main thread on Windows #74679

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
VSadov merged 2 commits into from
Aug 27, 2022
Merged

[NativeAOT] do not do shutdown for the main thread on Windows #74679

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ec736fa
do not do shutdown for the main thread
VSadov Aug 26, 2022
fbe4267
specialcase the thread that called RhpShutdown instead
VSadov Aug 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ uint32_t PalEventWrite(REGHANDLE arg1, const EVENT_DESCRIPTOR * arg2, uint32_t a
// Index for the fiber local storage of the attached thread pointer
static uint32_t g_flsIndex = FLS_OUT_OF_INDEXES;

static uint32_t g_mainThreadId = 0;

// This is called when each *fiber* is destroyed. When the home fiber of a thread is destroyed,
// it means that the thread itself is destroyed.
// Since we receive that notification outside of the Loader Lock, it allows us to safely acquire
Expand All @@ -50,7 +52,13 @@ void __stdcall FiberDetachCallback(void* lpFlsData)
if (lpFlsData != NULL)
{
// The current fiber is the home fiber of a thread, so the thread is shutting down
RuntimeThreadShutdown(lpFlsData);
// Do not do shutdown for the main thread though.
// other threads are terminated before the main one and could leave TLS locked,
Copy link
Member

@jkotas jkotas Aug 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not how shutdown works. The shutdown first terminates all threads except the thread that called ExitProcess. ExitProcess can be called by any thread.

Copy link
Member Author

@VSadov VSadov Aug 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a lot of vague information on the details of process/thread shutdown. It looks like in a completely broad sense it is hard to handle as there is always some crazy scenario such as someone starting new threads in a DLLMain detach handler.

I think we should strive for handling "common" cases.
However using NativeAOT in a library and calling ExitProcess from non-main thread would seem reasonable.

// so we will likely deadlock
if (GetCurrentThreadId() != g_mainThreadId)
{
RuntimeThreadShutdown(lpFlsData);
}
}
}

Expand Down Expand Up @@ -137,6 +145,8 @@ void InitializeCurrentProcessCpuCount()
// initialization and false on failure.
REDHAWK_PALEXPORT bool REDHAWK_PALAPI PalInit()
{
g_mainThreadId = GetCurrentThreadId();
Copy link
Member

@jkotas jkotas Aug 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The initialization can run on any thread when NativeAOT is used to produce libraries.

Copy link
Member Author

@VSadov VSadov Aug 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is inconvenient.

There is no easy way to identify the main thread.
Online samples suggest iterating through all threads and look for the earliest start time.

Copy link
Member Author

@VSadov VSadov Aug 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A part of the problem is that the thread object in NativeAOT lives directly in a TSL slot and thus is owned by the thread, so we must unlink the threads from the list before the thread dies and that requires taking a lock.


// We use fiber detach callbacks to run our thread shutdown code because the fiber detach
// callback is made without the OS loader lock
g_flsIndex = FlsAlloc(FiberDetachCallback);
Expand Down