Commit df59f217 authored by Shu-yu Guo's avatar Shu-yu Guo Committed by V8 LUCI CQ

[shared-struct] Fix external pointers to WaiterQueueNodes

WaiterQueueNodes as used by JS synchronization primitives are per-main
thread, and external pointer handles to those nodes are 1-1. That
1-1-ness is captured by each main thread Isolate having a
waiter_queue_node_external_pointer_ field.

The current logic is incorrect on unlock paths as the Isolate that
requested the unlock can point its own
waiter_queue_node_external_pointer_ to another Isolate's
WaiterQueueNode. This CL fixes this by having each WaiterQueueNode hold onto its own external pointer handle.

This CL also fixes an embarrassing bug where the WaiterQueueNode was not correctly dequeued on timeout.

Bug: v8:13189, v8:12547
Change-Id: I8db16ae6d653d2e71989ad003faae20fcee06a25
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3832298
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Reviewed-by: 's avatarJakob Kummerow <jkummerow@chromium.org>
Reviewed-by: 's avatarSamuel Groß <saelo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82601}
parent d9a44b75
...@@ -5843,17 +5843,14 @@ void Isolate::DetachFromSharedIsolate() { ...@@ -5843,17 +5843,14 @@ void Isolate::DetachFromSharedIsolate() {
} }
#ifdef V8_COMPRESS_POINTERS #ifdef V8_COMPRESS_POINTERS
ExternalPointerHandle ExternalPointerHandle Isolate::GetOrCreateWaiterQueueNodeExternalPointer() {
Isolate::InsertWaiterQueueNodeIntoSharedExternalPointerTable(Address node) {
DCHECK_NE(kNullAddress, node);
ExternalPointerHandle handle; ExternalPointerHandle handle;
if (waiter_queue_node_external_pointer_handle_ != if (waiter_queue_node_external_pointer_handle_ !=
kNullExternalPointerHandle) { kNullExternalPointerHandle) {
handle = waiter_queue_node_external_pointer_handle_; handle = waiter_queue_node_external_pointer_handle_;
shared_external_pointer_table().Set(handle, node, kWaiterQueueNodeTag);
} else { } else {
handle = shared_external_pointer_table().AllocateAndInitializeEntry( handle = shared_external_pointer_table().AllocateAndInitializeEntry(
node, kWaiterQueueNodeTag); kNullAddress, kWaiterQueueNodeTag);
waiter_queue_node_external_pointer_handle_ = handle; waiter_queue_node_external_pointer_handle_ = handle;
} }
DCHECK_NE(0, handle); DCHECK_NE(0, handle);
......
...@@ -1976,8 +1976,7 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory { ...@@ -1976,8 +1976,7 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
return &waiter_queue_node_external_pointer_handle_; return &waiter_queue_node_external_pointer_handle_;
} }
ExternalPointerHandle InsertWaiterQueueNodeIntoSharedExternalPointerTable( ExternalPointerHandle GetOrCreateWaiterQueueNodeExternalPointer();
Address node);
#endif // V8_COMPRESS_POINTERS #endif // V8_COMPRESS_POINTERS
struct PromiseHookFields { struct PromiseHookFields {
......
This diff is collapsed.
...@@ -145,8 +145,9 @@ class JSAtomicsMutex ...@@ -145,8 +145,9 @@ class JSAtomicsMutex
inline std::atomic<int32_t>* AtomicOwnerThreadIdPtr(); inline std::atomic<int32_t>* AtomicOwnerThreadIdPtr();
bool TryLockExplicit(std::atomic<StateT>* state, StateT& expected); static bool TryLockExplicit(std::atomic<StateT>* state, StateT& expected);
bool TryLockWaiterQueueExplicit(std::atomic<StateT>* state, StateT& expected); static bool TryLockWaiterQueueExplicit(std::atomic<StateT>* state,
StateT& expected);
V8_EXPORT_PRIVATE static void LockSlowPath(Isolate* requester, V8_EXPORT_PRIVATE static void LockSlowPath(Isolate* requester,
Handle<JSAtomicsMutex> mutex, Handle<JSAtomicsMutex> mutex,
...@@ -205,7 +206,14 @@ class JSAtomicsCondition ...@@ -205,7 +206,14 @@ class JSAtomicsCondition
static constexpr StateT kLockBitsMask = (1 << kLockBitsSize) - 1; static constexpr StateT kLockBitsMask = (1 << kLockBitsSize) - 1;
static constexpr StateT kWaiterQueueHeadMask = ~kLockBitsMask; static constexpr StateT kWaiterQueueHeadMask = ~kLockBitsMask;
bool TryLockWaiterQueueExplicit(std::atomic<StateT>* state, StateT& expected); static bool TryLockWaiterQueueExplicit(std::atomic<StateT>* state,
StateT& expected);
using DequeueAction =
std::function<detail::WaiterQueueNode*(detail::WaiterQueueNode**)>;
static detail::WaiterQueueNode* DequeueExplicit(
Isolate* requester, std::atomic<StateT>* state,
const DequeueAction& dequeue_action);
}; };
} // namespace internal } // namespace internal
......
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