Commit b1a8df5d authored by Shu-yu Guo's avatar Shu-yu Guo Committed by Commit Bot

Revert "[atomics] Fix critical section for Atomics.waitAsync"

This reverts commit de5f8614.

Reason for revert: TSAN https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20TSAN/b8851216731882090320/overview

Original change's description:
> [atomics] Fix critical section for Atomics.waitAsync
>
> Loading the value at the index for the futex wait should be protected by
> the waiterlist mutex for both sync and async waits.
>
> Bug: chromium:1194026
> Change-Id: Ie9896cab6828763ebb963f5ad96f264d57c9377f
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2796159
> Commit-Queue: Shu-yu Guo <syg@chromium.org>
> Reviewed-by: Marja Hölttä <marja@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#73753}

Bug: chromium:1194026
Change-Id: I63d5e224f11a35fd9c36d62d08ce642d3e6f64bf
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2797550
Auto-Submit: Shu-yu Guo <syg@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#73755}
parent 3d2f61fb
...@@ -517,6 +517,13 @@ FutexWaitListNode::FutexWaitListNode( ...@@ -517,6 +517,13 @@ FutexWaitListNode::FutexWaitListNode(
Utils::ToLocal(Handle<Context>::cast(native_context)); Utils::ToLocal(Handle<Context>::cast(native_context));
native_context_.Reset(v8_isolate, local_native_context); native_context_.Reset(v8_isolate, local_native_context);
native_context_.SetWeak(); native_context_.SetWeak();
// Add the Promise into the NativeContext's atomics_waitasync_promises set, so
// that the list keeps it alive.
Handle<OrderedHashSet> promises(native_context->atomics_waitasync_promises(),
isolate);
promises = OrderedHashSet::Add(isolate, promises, promise).ToHandleChecked();
native_context->set_atomics_waitasync_promises(*promises);
} }
template <typename T> template <typename T>
...@@ -529,108 +536,77 @@ Object FutexEmulation::WaitAsync(Isolate* isolate, ...@@ -529,108 +536,77 @@ Object FutexEmulation::WaitAsync(Isolate* isolate,
Factory* factory = isolate->factory(); Factory* factory = isolate->factory();
Handle<JSObject> result = factory->NewJSObject(isolate->object_function()); Handle<JSObject> result = factory->NewJSObject(isolate->object_function());
Handle<JSObject> promise_capability = factory->NewJSPromise();
enum { kNotEqual, kTimedOut, kAsync } result_kind;
FutexWaitListNode* node = nullptr;
{
// 16. Perform EnterCriticalSection(WL).
NoGarbageCollectionMutexGuard lock_guard(g_mutex.Pointer());
std::shared_ptr<BackingStore> backing_store =
array_buffer->GetBackingStore();
// 17. Let w be ! AtomicLoad(typedArray, i). std::shared_ptr<BackingStore> backing_store = array_buffer->GetBackingStore();
std::atomic<T>* p = reinterpret_cast<std::atomic<T>*>(
static_cast<int8_t*>(backing_store->buffer_start()) + addr);
if (p->load() != value) {
result_kind = kNotEqual;
} else if (use_timeout && rel_timeout_ns == 0) {
result_kind = kTimedOut;
} else {
result_kind = kAsync;
node = new FutexWaitListNode(backing_store, addr, promise_capability,
isolate);
g_wait_list.Pointer()->AddNode(node);
}
// Leaving the block collapses the following steps: // 17. Let w be ! AtomicLoad(typedArray, i).
// 18.a. Perform LeaveCriticalSection(WL). std::atomic<T>* p = reinterpret_cast<std::atomic<T>*>(
// 19.b. Perform LeaveCriticalSection(WL). static_cast<int8_t*>(backing_store->buffer_start()) + addr);
// 24. Perform LeaveCriticalSection(WL). if (p->load() != value) {
// 18. If v is not equal to w, then
// a. Perform LeaveCriticalSection(WL).
// ...
// c. Perform ! CreateDataPropertyOrThrow(resultObject, "async", false).
// d. Perform ! CreateDataPropertyOrThrow(resultObject, "value",
// "not-equal").
// e. Return resultObject.
CHECK(
JSReceiver::CreateDataProperty(isolate, result, factory->async_string(),
factory->false_value(), Just(kDontThrow))
.FromJust());
CHECK(JSReceiver::CreateDataProperty(
isolate, result, factory->value_string(),
factory->not_equal_string(), Just(kDontThrow))
.FromJust());
return *result;
}
if (use_timeout && rel_timeout_ns == 0) {
// 19. If t is 0 and mode is async, then
// ...
// b. Perform LeaveCriticalSection(WL).
// c. Perform ! CreateDataPropertyOrThrow(resultObject, "async", false).
// d. Perform ! CreateDataPropertyOrThrow(resultObject, "value",
// "timed-out").
// e. Return resultObject.
CHECK(
JSReceiver::CreateDataProperty(isolate, result, factory->async_string(),
factory->false_value(), Just(kDontThrow))
.FromJust());
CHECK(JSReceiver::CreateDataProperty(
isolate, result, factory->value_string(),
factory->timed_out_string(), Just(kDontThrow))
.FromJust());
return *result;
} }
switch (result_kind) { Handle<JSObject> promise_capability = factory->NewJSPromise();
case kNotEqual: FutexWaitListNode* node =
// 18. If v is not equal to w, then new FutexWaitListNode(backing_store, addr, promise_capability, isolate);
// ...
// c. Perform ! CreateDataPropertyOrThrow(resultObject, "async", false).
// d. Perform ! CreateDataPropertyOrThrow(resultObject, "value",
// "not-equal").
// e. Return resultObject.
CHECK(JSReceiver::CreateDataProperty(
isolate, result, factory->async_string(),
factory->false_value(), Just(kDontThrow))
.FromJust());
CHECK(JSReceiver::CreateDataProperty(
isolate, result, factory->value_string(),
factory->not_equal_string(), Just(kDontThrow))
.FromJust());
break;
case kTimedOut:
// 19. If t is 0 and mode is async, then
// ...
// c. Perform ! CreateDataPropertyOrThrow(resultObject, "async", false).
// d. Perform ! CreateDataPropertyOrThrow(resultObject, "value",
// "timed-out").
// e. Return resultObject.
CHECK(JSReceiver::CreateDataProperty(
isolate, result, factory->async_string(),
factory->false_value(), Just(kDontThrow))
.FromJust());
CHECK(JSReceiver::CreateDataProperty(
isolate, result, factory->value_string(),
factory->timed_out_string(), Just(kDontThrow))
.FromJust());
break;
case kAsync:
DCHECK_NOT_NULL(node);
// Add the Promise into the NativeContext's atomics_waitasync_promises
// set, so that the list keeps it alive.
Handle<NativeContext> native_context(isolate->native_context());
Handle<OrderedHashSet> promises(
native_context->atomics_waitasync_promises(), isolate);
promises = OrderedHashSet::Add(isolate, promises, promise_capability)
.ToHandleChecked();
native_context->set_atomics_waitasync_promises(*promises);
if (use_timeout) {
node->async_timeout_time_ = base::TimeTicks::Now() + rel_timeout;
auto task = std::make_unique<AsyncWaiterTimeoutTask>(
node->cancelable_task_manager_, node);
node->timeout_task_id_ = task->id();
node->task_runner_->PostNonNestableDelayedTask(
std::move(task), rel_timeout.InSecondsF());
}
// 26. Perform ! CreateDataPropertyOrThrow(resultObject, "async", true). {
// 27. Perform ! CreateDataPropertyOrThrow(resultObject, "value", NoGarbageCollectionMutexGuard lock_guard(g_mutex.Pointer());
// promiseCapability.[[Promise]]). g_wait_list.Pointer()->AddNode(node);
// 28. Return resultObject.
CHECK(JSReceiver::CreateDataProperty(
isolate, result, factory->async_string(), factory->true_value(),
Just(kDontThrow))
.FromJust());
CHECK(JSReceiver::CreateDataProperty(isolate, result,
factory->value_string(),
promise_capability, Just(kDontThrow))
.FromJust());
break;
} }
if (use_timeout) {
node->async_timeout_time_ = base::TimeTicks::Now() + rel_timeout;
auto task = std::make_unique<AsyncWaiterTimeoutTask>(
node->cancelable_task_manager_, node);
node->timeout_task_id_ = task->id();
node->task_runner_->PostNonNestableDelayedTask(std::move(task),
rel_timeout.InSecondsF());
}
// 26. Perform ! CreateDataPropertyOrThrow(resultObject, "async", true).
// 27. Perform ! CreateDataPropertyOrThrow(resultObject, "value",
// promiseCapability.[[Promise]]).
// 28. Return resultObject.
CHECK(JSReceiver::CreateDataProperty(isolate, result, factory->async_string(),
factory->true_value(), Just(kDontThrow))
.FromJust());
CHECK(JSReceiver::CreateDataProperty(isolate, result, factory->value_string(),
promise_capability, Just(kDontThrow))
.FromJust());
return *result; return *result;
} }
......
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