-
Couldn't load subscription status.
- Fork 5.2k
Enable sending activation to dispatch queue threads #102887
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.
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
janvorli
merged 11 commits into
dotnet:main
from
janvorli:enable-activation-sending-to-dispatch-queue-threads
Jun 27, 2024
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
577027e
Enable sending activation to dispatch queue threads
janvorli 6ba16e0
Workaround until CI can switch to newer xcode
janvorli 131de5d
Add unmasking activation signal on foreign threads
janvorli d90911f
Add test to verify the fix
janvorli c2b7263
Fix build break
janvorli fcd947e
The test should be Apple only
janvorli 51c89fc
Build the native part of the test on Apple only
janvorli b38eae4
Fix reverse polarity assert in nativeaot
janvorli c3cc9f1
Change usage of libdispatch to libSystem
janvorli fa2d9c9
Make sure the testing is performed only on OS that supports the new API
janvorli 8121f7d
Add RequiresProcessIsolation for CLRTestTargetUnsupported
janvorli File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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
27
src/tests/Regressions/coreclr/GitHub_102887/nativetest102887.cpp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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
17
src/tests/Regressions/coreclr/GitHub_102887/test102887.csproj
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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> |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this going to work for iOS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should, the new API supports iOS too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is
dlopenof a random library fine wrt. app store rules?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know, but I hope that we will be able to remove the dlopen very soon, it is a matter of upgrading macOS on our CI machines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @ivanpovazan
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file is available at
/usr/lib/system/introspection/libdispatch.dylib:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is interesting. I also have the library in the /usr/lib/system/introspection and not in the /usr/lib/system. But when I run a simple c app that invokes that API under lldb and list the images in the process, it shows the path I have used:
The dlopen works no matter whether I use the /usr/lib/system/libdispatch.dylib or /usr/lib/system/introspection/libdispatch.dylib. However, if I load the one in the /usr/lib/system/introspection, then lldb shows many warnings like
So it seems that macOS has some redirection there and the path I have in the PR is the right one to use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dylibs don't actually exist on disk in macOS anymore, they're in a system-level dyld cache, which is why 'find' or 'ls' won't find them. dlopen knows to look in the cache though, so that's why the path just works.
Ref: https://mjtsai.com/blog/2020/06/26/reverse-engineering-macos-11-0/
One way to find out which dylib a symbol is supposed to be in is by looking in the Xcode's SDKS:
This shows that the symbol is also in libSystem.B.
One interesting thing shows up when grepping in the iOS SDK:
The symbol is only available in libSystem, not libdispatch (the same happens in tvOS too).
My recommendation would be to dlopen libSystem instead ("/usr/lib/libSystem.dylib"), hopefully that will work on all platforms.
Ideally you'd be able to remove the call to dlopen, that would reliably work on all platforms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I am really waiting for the CI machines to get updated.
I'll give the libSystem a try, hopefully it would work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rolfbjarne thank you for the suggestion, the libSystem works too. I'll update this PR.