Commit 3a44f269 authored by Clemens Backes's avatar Clemens Backes Committed by V8 LUCI CQ

[base] Avoid pthread_rwlock_t on Mac

pthread_rwlock_t can deadlock on Mac if signals are sent to the process
in the wrong moment. Since we use processes e.g. for sampling profiling
(in both d8 and in Chrome), we hence cannot safely use pthread_rwlock_t
on Mac. Instead, fall back to a non-shared pthread_mutex_t.

Interestingly, this shows no measurable performance impact in Wasm
compilation on my MBP.

R=mlippautz@chromium.org

Bug: v8:11399
Change-Id: Ie8bfd5288bba8c4f3315ee4502b39b59d39c9bbd
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3060480Reviewed-by: 's avatarVictor Gomes <victorgomes@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#76015}
parent e82b368b
......@@ -218,6 +218,37 @@ bool RecursiveMutex::TryLock() {
return true;
}
#if V8_OS_MACOSX
SharedMutex::SharedMutex() { InitializeNativeHandle(&native_handle_); }
SharedMutex::~SharedMutex() { DestroyNativeHandle(&native_handle_); }
void SharedMutex::LockShared() { LockExclusive(); }
void SharedMutex::LockExclusive() {
DCHECK(TryHoldSharedMutex(this));
LockNativeHandle(&native_handle_);
}
void SharedMutex::UnlockShared() { UnlockExclusive(); }
void SharedMutex::UnlockExclusive() {
DCHECK(TryReleaseSharedMutex(this));
UnlockNativeHandle(&native_handle_);
}
bool SharedMutex::TryLockShared() { return TryLockExclusive(); }
bool SharedMutex::TryLockExclusive() {
DCHECK(SharedMutexNotHeld(this));
if (!TryLockNativeHandle(&native_handle_)) return false;
DCHECK(TryHoldSharedMutex(this));
return true;
}
#else // !V8_OS_MACOSX
SharedMutex::SharedMutex() { pthread_rwlock_init(&native_handle_, nullptr); }
SharedMutex::~SharedMutex() {
......@@ -266,6 +297,8 @@ bool SharedMutex::TryLockExclusive() {
return result;
}
#endif // !V8_OS_MACOSX
#elif V8_OS_WIN
Mutex::Mutex() : native_handle_(SRWLOCK_INIT) {
......
......@@ -265,7 +265,12 @@ class V8_BASE_EXPORT SharedMutex final {
private:
// The implementation-defined native handle type.
#if V8_OS_POSIX
#if V8_OS_MACOSX
// pthread_rwlock_t is broken on MacOS when signals are being sent to the
// process (see https://crbug.com/v8/11399). Until Apple fixes that in the OS,
// we have to fall back to a non-shared mutex.
using NativeHandle = pthread_mutex_t;
#elif V8_OS_POSIX
using NativeHandle = pthread_rwlock_t;
#elif V8_OS_WIN
using NativeHandle = SRWLOCK;
......
......@@ -117,9 +117,6 @@
'test-serialize/CustomSnapshotDataBlobImmortalImmovableRoots': [PASS, ['mode == debug', SKIP]],
'test-parsing/ObjectRestNegativeTestSlow': [PASS, ['mode == debug', SKIP]],
# pthread_rwlock_t combined with signals is broken on Mac (https://crbug.com/v8/11399).
'signals-and-mutexes/SignalsPlusSharedMutexes': [PASS, ['system == macos', SKIP]],
# Tests that need to run sequentially (e.g. due to memory consumption).
'test-accessors/HandleScopePop': [PASS, HEAVY],
'test-api/FastApiCalls': [PASS, HEAVY],
......
......@@ -19,6 +19,8 @@ namespace sampler {
// https://stackoverflow.com/questions/22643374/deadlock-with-pthread-rwlock-t-and-signals
// This test reproduces it, and can be used to test if this problem is fixed in
// future Mac releases.
// Note: For now, we fall back to using pthread_mutex_t to implement SharedMutex
// on Mac, so this test succeeds.
#ifdef USE_SIGNALS
......
......@@ -42,12 +42,6 @@
'debugger/get-possible-breakpoints-class-fields': [SKIP],
}], # 'system == android'
##############################################################################
['system == macos', {
# Bug(v8:11399): Deadlock on Mac.
'cpu-profiler/console-profile-wasm': [SKIP],
}],
##############################################################################
['variant != default', {
# Issue 6167.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment