Skip to content

Commit

Permalink
Enable sending activation to dispatch queue threads (#102887)
Browse files Browse the repository at this point in the history
* Enable sending activation to dispatch queue threads

Until recently, it was not possible to send activation signal to Apple
dispatch queue threads using pthread_kill. So in case a .NET code was running
on such a thread, it could end up blocking runtime suspension in some cases.
Recently, Apple has implemented a new API to enable sending specific signals
to the dispatch queues, the dispatch_allow_send_signals.

This change adds a call to that API to the PAL initialization code.

* Workaround until CI can switch to newer xcode

* Add unmasking activation signal on foreign threads

The added coreclr test has revealed that while the signal could be sent now,
the inject_activation_handler was not being invoked on the dispatch queue
thread. The problem was that all signals are masked by default there.

* Add test to verify the fix

The test verifies that .NET code running on a dispatch queue thread in
a tight loop can get the activation injected.

* Fix build break

* The test should be Apple only

* Build the native part of the test on Apple only

* Fix reverse polarity assert in nativeaot

* Change usage of libdispatch to libSystem

* Make sure the testing is performed only on OS that supports the new API

* Add RequiresProcessIsolation for CLRTestTargetUnsupported
  • Loading branch information
janvorli authored Jun 27, 2024
1 parent f7636f4 commit 742bae8
Show file tree
Hide file tree
Showing 8 changed files with 206 additions and 18 deletions.
35 changes: 34 additions & 1 deletion src/coreclr/nativeaot/Runtime/unix/PalRedhawkUnix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,16 @@ extern "C" void RaiseFailFastException(PEXCEPTION_RECORD arg1, PCONTEXT arg2, ui
abort();
}

static void UnmaskActivationSignal()
{
sigset_t signal_set;
sigemptyset(&signal_set);
sigaddset(&signal_set, INJECT_ACTIVATION_SIGNAL);

int sigmaskRet = pthread_sigmask(SIG_UNBLOCK, &signal_set, NULL);
_ASSERTE(sigmaskRet == 0);
}

static void TimeSpecAdd(timespec* time, uint32_t milliseconds)
{
uint64_t nsec = time->tv_nsec + (uint64_t)milliseconds * tccMilliSecondsToNanoSeconds;
Expand Down Expand Up @@ -501,6 +511,8 @@ extern "C" void PalAttachThread(void* thread)
#else
tls_destructionMonitor.SetThread(thread);
#endif

UnmaskActivationSignal();
}

// Detach thread from OS notifications.
Expand Down Expand Up @@ -1034,6 +1046,27 @@ REDHAWK_PALEXPORT UInt32_BOOL REDHAWK_PALAPI PalRegisterHijackCallback(_In_ PalH
ASSERT(g_pHijackCallback == NULL);
g_pHijackCallback = callback;

#ifdef __APPLE__
void *libSystem = dlopen("/usr/lib/libSystem.dylib", RTLD_LAZY);
if (libSystem != NULL)
{
int (*dispatch_allow_send_signals_ptr)(int) = (int (*)(int))dlsym(libSystem, "dispatch_allow_send_signals");
if (dispatch_allow_send_signals_ptr != NULL)
{
int status = dispatch_allow_send_signals_ptr(INJECT_ACTIVATION_SIGNAL);
_ASSERTE(status == 0);
}
}

// TODO: Once our CI tools can get upgraded to xcode >= 15.3, replace the code above by this:
// if (__builtin_available(macOS 14.4, iOS 17.4, tvOS 17.4, *))
// {
// // Allow sending the activation signal to dispatch queue threads
// int status = dispatch_allow_send_signals(INJECT_ACTIVATION_SIGNAL);
// _ASSERTE(status == 0);
// }
#endif // __APPLE__

return AddSignalHandler(INJECT_ACTIVATION_SIGNAL, ActivationHandler, &g_previousActivationHandler);
}

Expand All @@ -1053,7 +1086,7 @@ REDHAWK_PALEXPORT void REDHAWK_PALAPI PalHijack(HANDLE hThread, _In_opt_ void* p
if ((status == EAGAIN)
|| (status == ESRCH)
#ifdef __APPLE__
// On Apple, pthread_kill is not allowed to be sent to dispatch queue threads
// On Apple, pthread_kill is not allowed to be sent to dispatch queue threads on macOS older than 14.4 or iOS/tvOS older than 17.4
|| (status == ENOTSUP)
#endif
)
Expand Down
76 changes: 59 additions & 17 deletions src/coreclr/pal/src/exception/signal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ SET_DEFAULT_DEBUG_CHANNEL(EXCEPT); // some headers have code with asserts, so do

#include <errno.h>
#include <signal.h>
#include <dlfcn.h>

#if !HAVE_MACH_EXCEPTIONS
#include "pal/init.h"
Expand Down Expand Up @@ -74,6 +75,9 @@ static void sigterm_handler(int code, siginfo_t *siginfo, void *context);
#ifdef INJECT_ACTIVATION_SIGNAL
static void inject_activation_handler(int code, siginfo_t *siginfo, void *context);
extern void* g_InvokeActivationHandlerReturnAddress;
#ifdef __APPLE__
bool g_canSendSignalToDispatchQueueThreads = false;
#endif // __APPLE__
#endif

static void sigill_handler(int code, siginfo_t *siginfo, void *context);
Expand Down Expand Up @@ -239,6 +243,27 @@ BOOL SEHInitializeSignals(CorUnix::CPalThread *pthrCurrent, DWORD flags)
#ifdef INJECT_ACTIVATION_SIGNAL
if (flags & PAL_INITIALIZE_REGISTER_ACTIVATION_SIGNAL)
{
#ifdef __APPLE__
void *libSystem = dlopen("/usr/lib/libSystem.dylib", RTLD_LAZY);
if (libSystem != NULL)
{
int (*dispatch_allow_send_signals_ptr)(int) = (int (*)(int))dlsym(libSystem, "dispatch_allow_send_signals");
if (dispatch_allow_send_signals_ptr != NULL)
{
int st = dispatch_allow_send_signals_ptr(INJECT_ACTIVATION_SIGNAL);
g_canSendSignalToDispatchQueueThreads = (st == 0);
}
}

// TODO: Once our CI tools can get upgraded to xcode >= 15.4, replace the code above by this:
// if (__builtin_available(macOS 14.4, *))
// {
// // Allow sending the activation signal to dispatch queue threads
// int st = dispatch_allow_send_signals(INJECT_ACTIVATION_SIGNAL);
// g_canSendSignalToDispatchQueueThreads = (st == 0);
// }
#endif // __APPLE__

handle_signal(INJECT_ACTIVATION_SIGNAL, inject_activation_handler, &g_previous_activation);
g_registered_activation_handler = true;
}
Expand Down Expand Up @@ -471,6 +496,32 @@ static void sigfpe_handler(int code, siginfo_t *siginfo, void *context)
invoke_previous_action(&g_previous_sigfpe, code, siginfo, context);
}

void UnmaskActivationSignal()
{
sigset_t signal_set;
sigemptyset(&signal_set);
sigaddset(&signal_set, INJECT_ACTIVATION_SIGNAL);

int sigmaskRet = pthread_sigmask(SIG_UNBLOCK, &signal_set, NULL);
if (sigmaskRet != 0)
{
ASSERT("pthread_sigmask failed; error number is %d\n", sigmaskRet);
}
}

void MaskActivationSignal()
{
sigset_t signal_set;
sigemptyset(&signal_set);
sigaddset(&signal_set, INJECT_ACTIVATION_SIGNAL);

int sigmaskRet = pthread_sigmask(SIG_BLOCK, &signal_set, NULL);
if (sigmaskRet != 0)
{
ASSERT("pthread_sigmask failed; error number is %d\n", sigmaskRet);
}
}

#if !HAVE_MACH_EXCEPTIONS

/*++
Expand All @@ -493,24 +544,12 @@ extern "C" void signal_handler_worker(int code, siginfo_t *siginfo, void *contex
// to correctly fill in this value.

// Unmask the activation signal now that we are running on the original stack of the thread
sigset_t signal_set;
sigemptyset(&signal_set);
sigaddset(&signal_set, INJECT_ACTIVATION_SIGNAL);

int sigmaskRet = pthread_sigmask(SIG_UNBLOCK, &signal_set, NULL);
if (sigmaskRet != 0)
{
ASSERT("pthread_sigmask failed; error number is %d\n", sigmaskRet);
}
UnmaskActivationSignal();

returnPoint->returnFromHandler = common_signal_handler(code, siginfo, context, 2, (size_t)0, (size_t)siginfo->si_addr);

// We are going to return to the alternate stack, so block the activation signal again
sigmaskRet = pthread_sigmask(SIG_BLOCK, &signal_set, NULL);
if (sigmaskRet != 0)
{
ASSERT("pthread_sigmask failed; error number is %d\n", sigmaskRet);
}
MaskActivationSignal();

RtlRestoreContext(&returnPoint->context, NULL);
}
Expand Down Expand Up @@ -886,10 +925,13 @@ PAL_ERROR InjectActivationInternal(CorUnix::CPalThread* pThread)
// the process exits.

#ifdef __APPLE__
// On Apple, pthread_kill is not allowed to be sent to dispatch queue threads
if (status == ENOTSUP)
if (!g_canSendSignalToDispatchQueueThreads)
{
return ERROR_NOT_SUPPORTED;
// On macOS older than 14.4, pthread_kill is not allowed to sent a signal to dispatch queue threads
if (status == ENOTSUP)
{
return ERROR_NOT_SUPPORTED;
}
}
#endif

Expand Down
10 changes: 10 additions & 0 deletions src/coreclr/pal/src/include/pal/signal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,4 +117,14 @@ Function :
--*/
void SEHCleanupSignals();

/*++
Function :
UnmaskActivationSignal
Unmask the INJECT_ACTIVATION_SIGNAL for the current thread
(no parameters, no return value)
--*/
void UnmaskActivationSignal();

#endif /* _PAL_SIGNAL_HPP_ */
3 changes: 3 additions & 0 deletions src/coreclr/pal/src/init/sxs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ AllocatePalThread(CPalThread **ppThread)

PROCAddThread(pThread, pThread);

// Unmask the activation signal so that GC can suspend this thread
UnmaskActivationSignal();

exit:
*ppThread = pThread;
return palError;
Expand Down
9 changes: 9 additions & 0 deletions src/tests/Regressions/coreclr/GitHub_102887/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# The test is valid only on Apple
if (CLR_CMAKE_TARGET_APPLE)
include_directories(${INC_PLATFORM_DIR})

add_library(nativetest102887 SHARED nativetest102887.cpp)

# add the install targets
install (TARGETS nativetest102887 DESTINATION bin)
endif ()
27 changes: 27 additions & 0 deletions src/tests/Regressions/coreclr/GitHub_102887/nativetest102887.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#include <platformdefines.h>

#include <dlfcn.h>
#include <dispatch/dispatch.h>
#include <dispatch/queue.h>

extern "C" DLL_EXPORT void StartDispatchQueueThread(void (*work)(void* args))
{
dispatch_queue_global_t q = dispatch_get_global_queue(QOS_CLASS_UTILITY, 0);
dispatch_async_f(q, NULL, work);
}

extern "C" DLL_EXPORT bool SupportsSendingSignalsToDispatchQueueThread()
{
void *libSystem = dlopen("/usr/lib/libSystem.dylib", RTLD_LAZY);
bool result = false;
if (libSystem != NULL)
{
int (*dispatch_allow_send_signals_ptr)(int) = (int (*)(int))dlsym(libSystem, "dispatch_allow_send_signals");
result = (dispatch_allow_send_signals_ptr != NULL);
}

return result;
}
47 changes: 47 additions & 0 deletions src/tests/Regressions/coreclr/GitHub_102887/test102887.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
using System;
using System.Runtime.InteropServices;
using System.Threading;
using Xunit;

public class Test102887
{
delegate void DispatchQueueWork(IntPtr args);
[DllImport("nativetest102887")]
private static extern void StartDispatchQueueThread(DispatchQueueWork start);

[DllImport("nativetest102887")]
[return: MarshalAs(UnmanagedType.Bool)]
private static extern bool SupportsSendingSignalsToDispatchQueueThread();

static volatile int s_cnt;
static ManualResetEvent s_workStarted = new ManualResetEvent(false);

private static void RunOnDispatchQueueThread(IntPtr args)
{
s_workStarted.Set();
while (true)
{
s_cnt++;
}
}

[Fact]
public static void TestEntryPoint()
{
// Skip the test if the current OS doesn't support sending signals to the dispatch queue threads
if (SupportsSendingSignalsToDispatchQueueThread())
{
Console.WriteLine("Sending signals to dispatch queue thread is supported, testing it now");
StartDispatchQueueThread(RunOnDispatchQueueThread);
s_workStarted.WaitOne();

for (int i = 0; i < 100; i++)
{
GC.Collect(2);
}
}
}
}

17 changes: 17 additions & 0 deletions src/tests/Regressions/coreclr/GitHub_102887/test102887.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<!-- Needed for CLRTestTargetUnsupported -->
<RequiresProcessIsolation>true</RequiresProcessIsolation>
<CLRTestTargetUnsupported Condition="'$(TargetsOSX)' != 'true' and '$(TargetsAppleMobile)' != 'true'">true</CLRTestTargetUnsupported>
<CLRTestPriority>1</CLRTestPriority>
</PropertyGroup>
<ItemGroup>
<Compile Include="test102887.cs" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="$(TestSourceDir)Common/CoreCLRTestLibrary/CoreCLRTestLibrary.csproj" />
</ItemGroup>
<ItemGroup>
<CMakeProjectReference Include="CMakeLists.txt" />
</ItemGroup>
</Project>

0 comments on commit 742bae8

Please sign in to comment.