Skip to content

Commit

Permalink
Fix incorrect nativeaot event thread / sequence number on shutdown (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
elinor-fung authored Jul 20, 2023
1 parent b59d9f1 commit d672bcc
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 155 deletions.
2 changes: 2 additions & 0 deletions src/coreclr/nativeaot/Runtime/DisabledEventPipeInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,7 @@ void DiagnosticServerAdapter_PauseForDiagnosticsMonitor() {}

void EventPipeAdapter_FinishInitialize() {}

void EventPipe_ThreadShutdown() { }

void EventPipeAdapter_Shutdown() {}
bool DiagnosticServerAdapter_Shutdown() { return false; }
9 changes: 2 additions & 7 deletions src/coreclr/nativeaot/Runtime/EnabledEventPipeInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,14 @@
#include "eventpipeadapter.h"
#include "diagnosticserveradapter.h"

#include "gcenv.h"
#include "regdisplay.h"
#include "StackFrameIterator.h"
#include "thread.h"
#include "SpinLock.h"

void EventPipeAdapter_Initialize() { EventPipeAdapter::Initialize(); }

bool DiagnosticServerAdapter_Initialize() { return DiagnosticServerAdapter::Initialize(); }
void DiagnosticServerAdapter_PauseForDiagnosticsMonitor() { DiagnosticServerAdapter::PauseForDiagnosticsMonitor();}


void EventPipeAdapter_FinishInitialize() { EventPipeAdapter::FinishInitialize(); }

void EventPipe_ThreadShutdown() { ep_rt_aot_thread_exited(); }

void EventPipeAdapter_Shutdown() { EventPipeAdapter::Shutdown(); }
bool DiagnosticServerAdapter_Shutdown() { return DiagnosticServerAdapter::Shutdown(); }
2 changes: 2 additions & 0 deletions src/coreclr/nativeaot/Runtime/EventPipeInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ void DiagnosticServerAdapter_PauseForDiagnosticsMonitor();

void EventPipeAdapter_FinishInitialize();

void EventPipe_ThreadShutdown();

void EventPipeAdapter_Shutdown();
bool DiagnosticServerAdapter_Shutdown();

Expand Down
73 changes: 40 additions & 33 deletions src/coreclr/nativeaot/Runtime/eventpipe/ep-rt-aot.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,7 @@
#include <unistd.h>
#endif

// The regdisplay.h, StackFrameIterator.h, and thread.h includes are present only to access the Thread
// class and can be removed if it turns out that the required ep_rt_thread_handle_t can be
// implemented in some manner that doesn't rely on the Thread class.

#include "gcenv.h"
#include "regdisplay.h"
#include "StackFrameIterator.h"
#include "thread.h"
#include "holder.h"
#include "SpinLock.h"

#ifndef DIRECTORY_SEPARATOR_CHAR
#ifdef TARGET_UNIX
Expand All @@ -36,14 +27,6 @@
#endif // TARGET_UNIX
#endif

#ifdef TARGET_UNIX
// Per module (1 for NativeAOT), key that will be used to implement TLS in Unix
pthread_key_t eventpipe_tls_key;
__thread EventPipeThreadHolder* eventpipe_tls_instance;
#else
thread_local EventPipeAotThreadHolderTLS EventPipeAotThreadHolderTLS::g_threadHolderTLS;
#endif

// Uses _rt_aot_lock_internal_t that has CrstStatic as a field
// This is initialized at the beginning and EventPipe library requires the lock handle to be maintained by the runtime
ep_rt_lock_handle_t _ep_rt_aot_config_lock_handle;
Expand Down Expand Up @@ -181,6 +164,46 @@ ep_rt_aot_diagnostics_command_line_get (void)
#endif
}

namespace
{
#ifdef TARGET_UNIX
__thread EventPipeThreadHolder* eventpipe_tls_instance;
#else
thread_local EventPipeThreadHolder* eventpipe_tls_instance;
#endif

void free_thread_holder ()
{
EventPipeThreadHolder *thread_holder = eventpipe_tls_instance;
if (thread_holder != NULL) {
thread_holder_free_func (thread_holder);
eventpipe_tls_instance = NULL;
}
}
}

EventPipeThread* ep_rt_aot_thread_get (void)
{
EventPipeThreadHolder *thread_holder = eventpipe_tls_instance;
return thread_holder ? ep_thread_holder_get_thread (thread_holder) : NULL;
}

EventPipeThread* ep_rt_aot_thread_get_or_create (void)
{
EventPipeThread *thread = ep_rt_thread_get ();
if (thread != NULL)
return thread;

eventpipe_tls_instance = thread_holder_alloc_func ();
return ep_thread_holder_get_thread (eventpipe_tls_instance);
}

void
ep_rt_aot_thread_exited (void)
{
free_thread_holder ();
}

uint32_t
ep_rt_aot_atomic_inc_uint32_t (volatile uint32_t *value)
{
Expand Down Expand Up @@ -679,29 +702,13 @@ ep_rt_aot_volatile_store_ptr_without_barrier (
VolatileStoreWithoutBarrier<void *> ((void **)ptr, value);
}

void unix_tls_callback_fn(void *value)
{
if (value) {
// we need to do the unallocation here
EventPipeThreadHolder *thread_holder_old = static_cast<EventPipeThreadHolder*>(value);
// @TODO - inline
thread_holder_free_func (thread_holder_old);
value = NULL;
}
}

void ep_rt_aot_init (void)
{
extern ep_rt_lock_handle_t _ep_rt_aot_config_lock_handle;
extern CrstStatic _ep_rt_aot_config_lock;

_ep_rt_aot_config_lock_handle.lock = &_ep_rt_aot_config_lock;
_ep_rt_aot_config_lock_handle.lock->InitNoThrow (CrstType::CrstEventPipeConfig);

// Initialize the pthread key used for TLS in Unix
#ifdef TARGET_UNIX
pthread_key_create(&eventpipe_tls_key, unix_tls_callback_fn);
#endif
}

bool ep_rt_aot_lock_acquire (ep_rt_lock_handle_t *lock)
Expand Down
105 changes: 5 additions & 100 deletions src/coreclr/nativeaot/Runtime/eventpipe/ep-rt-aot.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include <ctype.h> // For isspace
#ifdef TARGET_UNIX
#include <sys/time.h>
#include <pthread.h>
#endif

#include <eventpipe/ep-rt-config.h>
Expand Down Expand Up @@ -51,10 +50,7 @@
#define _TEXT(s) #s
#define STRINGIFY(s) _TEXT(s)

#ifdef TARGET_UNIX
extern pthread_key_t eventpipe_tls_key;
extern __thread EventPipeThreadHolder* eventpipe_tls_instance;
#endif
extern void ep_rt_aot_thread_exited (void);

// shipping criteria: no EVENTPIPE-NATIVEAOT-TODO left in the codebase
// TODO: The NativeAOT ALIGN_UP is defined in a tangled manner that generates linker errors if
Expand Down Expand Up @@ -1542,46 +1538,6 @@ thread_holder_free_func (EventPipeThreadHolder * thread_holder)
}
}

class EventPipeAotThreadHolderTLS {
public:
EventPipeAotThreadHolderTLS ()
{
STATIC_CONTRACT_NOTHROW;
}

~EventPipeAotThreadHolderTLS ()
{
STATIC_CONTRACT_NOTHROW;

if (m_threadHolder) {
thread_holder_free_func (m_threadHolder);
m_threadHolder = NULL;
}
}

static inline EventPipeThreadHolder * getThreadHolder ()
{
STATIC_CONTRACT_NOTHROW;
return g_threadHolderTLS.m_threadHolder;
}

static inline EventPipeThreadHolder * createThreadHolder ()
{
STATIC_CONTRACT_NOTHROW;

if (g_threadHolderTLS.m_threadHolder) {
thread_holder_free_func (g_threadHolderTLS.m_threadHolder);
g_threadHolderTLS.m_threadHolder = NULL;
}
g_threadHolderTLS.m_threadHolder = thread_holder_alloc_func ();
return g_threadHolderTLS.m_threadHolder;
}

private:
EventPipeThreadHolder *m_threadHolder;
static thread_local EventPipeAotThreadHolderTLS g_threadHolderTLS;
};

static
void
ep_rt_thread_setup (void)
Expand All @@ -1594,57 +1550,15 @@ ep_rt_thread_setup (void)
// EP_ASSERT (thread_handle != NULL);
}

#ifdef TARGET_UNIX
static
inline
EventPipeThreadHolder *
pthread_getThreadHolder (void)
{
void *value = eventpipe_tls_instance;
if (value) {
EventPipeThreadHolder *thread_holder = static_cast<EventPipeThreadHolder*>(value);
return thread_holder;
}
return NULL;
}

static
inline
EventPipeThreadHolder *
pthread_createThreadHolder (void)
{
void *value = eventpipe_tls_instance;
if (value) {
// we need to do the unallocation here
EventPipeThreadHolder *thread_holder_old = static_cast<EventPipeThreadHolder*>(value);
thread_holder_free_func(thread_holder_old);
eventpipe_tls_instance = NULL;

value = NULL;
}
EventPipeThreadHolder *instance = thread_holder_alloc_func();
if (instance){
// We need to know when the thread is no longer in use to clean up EventPipeThreadHolder instance and will use pthread destructor function to get notification when that happens.
pthread_setspecific(eventpipe_tls_key, instance);
eventpipe_tls_instance = instance;
}
return instance;
}
#endif

static
inline
EventPipeThread *
ep_rt_thread_get (void)
{
STATIC_CONTRACT_NOTHROW;

#ifdef TARGET_UNIX
EventPipeThreadHolder *thread_holder = pthread_getThreadHolder ();
#else
EventPipeThreadHolder *thread_holder = EventPipeAotThreadHolderTLS::getThreadHolder ();
#endif
return thread_holder ? ep_thread_holder_get_thread (thread_holder) : NULL;
extern EventPipeThread* ep_rt_aot_thread_get (void);
return ep_rt_aot_thread_get ();
}

static
Expand All @@ -1654,17 +1568,8 @@ ep_rt_thread_get_or_create (void)
{
STATIC_CONTRACT_NOTHROW;

#ifdef TARGET_UNIX
EventPipeThreadHolder *thread_holder = pthread_getThreadHolder ();
if (!thread_holder)
thread_holder = pthread_createThreadHolder ();
#else
EventPipeThreadHolder *thread_holder = EventPipeAotThreadHolderTLS::getThreadHolder ();
if (!thread_holder)
thread_holder = EventPipeAotThreadHolderTLS::createThreadHolder ();
#endif

return ep_thread_holder_get_thread (thread_holder);
extern EventPipeThread* ep_rt_aot_thread_get_or_create (void);
return ep_rt_aot_thread_get_or_create ();
}

static
Expand Down
15 changes: 0 additions & 15 deletions src/coreclr/nativeaot/Runtime/eventpipeadaptertypes.h

This file was deleted.

4 changes: 4 additions & 0 deletions src/coreclr/nativeaot/Runtime/startup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,10 @@ void RuntimeThreadShutdown(void* thread)
#endif

ThreadStore::DetachCurrentThread();

#ifdef FEATURE_PERFTRACING
EventPipe_ThreadShutdown();
#endif
}

extern "C" bool RhInitialize()
Expand Down

0 comments on commit d672bcc

Please sign in to comment.