Atomics.wait() throws on deadlock proof of concept
| diff --git a/deps/v8/include/v8.h b/deps/v8/include/v8.h | |
| index eaac6db0a17d..1f5feff6f2d3 100644 | |
| --- a/deps/v8/include/v8.h | |
| +++ b/deps/v8/include/v8.h | |
| @@ -7361,6 +7361,21 @@ class V8_EXPORT Isolate { | |
| void RemoveCallCompletedCallback( | |
| DeprecatedCallCompletedCallback callback)); | |
| + enum AtomicsWaitEvent { | |
| + kAtomicsStartWait, | |
| + kAtomicsEndWait | |
| + }; | |
| + | |
| + typedef void (*AtomicsWaitCallback)(enum AtomicsWaitEvent event, | |
| + Local<SharedArrayBuffer> array_buffer, | |
| + size_t offset_in_bytes, | |
| + int32_t value, | |
| + double timeout_in_ms, | |
| + bool* waiting, | |
| + void* data); | |
| + | |
| + void SetAtomicsWaitCallback(AtomicsWaitCallback callback, void* data); | |
| + | |
| /** | |
| * Experimental: Set the PromiseHook callback for various promise | |
| * lifecycle events. | |
| diff --git a/deps/v8/src/api.cc b/deps/v8/src/api.cc | |
| index 671960a0b729..caaf5e69fe92 100644 | |
| --- a/deps/v8/src/api.cc | |
| +++ b/deps/v8/src/api.cc | |
| @@ -8787,6 +8787,11 @@ void Isolate::RemoveCallCompletedCallback( | |
| reinterpret_cast<CallCompletedCallback>(callback)); | |
| } | |
| +void Isolate::SetAtomicsWaitCallback(AtomicsWaitCallback callback, void* data) { | |
| + i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this); | |
| + isolate->SetAtomicsWaitCallback(callback, data); | |
| +} | |
| + | |
| void Isolate::SetPromiseHook(PromiseHook hook) { | |
| i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this); | |
| isolate->SetPromiseHook(hook); | |
| diff --git a/deps/v8/src/futex-emulation.cc b/deps/v8/src/futex-emulation.cc | |
| index 63ad213f8d04..b7a4d24e4ab7 100644 | |
| --- a/deps/v8/src/futex-emulation.cc | |
| +++ b/deps/v8/src/futex-emulation.cc | |
| @@ -117,6 +117,17 @@ Object* FutexEmulation::Wait(Isolate* isolate, | |
| base::TimeTicks timeout_time = start_time + rel_timeout; | |
| base::TimeTicks current_time = start_time; | |
| + isolate->RunAtomicsWaitCallback(v8::Isolate::kAtomicsStartWait, | |
| + array_buffer, | |
| + addr, | |
| + value, | |
| + use_timeout ? rel_timeout_ms : -1.0, | |
| + &node->waiting_); | |
| + | |
| + if (isolate->has_scheduled_exception()) { | |
| + return isolate->PromoteScheduledException(); | |
| + } | |
| + | |
| wait_list_.Pointer()->AddNode(node); | |
| Object* result; | |
| @@ -183,8 +194,20 @@ Object* FutexEmulation::Wait(Isolate* isolate, | |
| } | |
| wait_list_.Pointer()->RemoveNode(node); | |
| + | |
| + isolate->RunAtomicsWaitCallback(v8::Isolate::kAtomicsEndWait, | |
| + array_buffer, | |
| + addr, | |
| + value, | |
| + use_timeout ? rel_timeout_ms : -1.0, | |
| + &node->waiting_); | |
| + | |
| node->waiting_ = false; | |
| + if (isolate->has_scheduled_exception()) { | |
| + result = isolate->PromoteScheduledException(); | |
| + } | |
| + | |
| return result; | |
| } | |
| diff --git a/deps/v8/src/isolate.cc b/deps/v8/src/isolate.cc | |
| index 48f5b30bd2f9..33e856842fd4 100644 | |
| --- a/deps/v8/src/isolate.cc | |
| +++ b/deps/v8/src/isolate.cc | |
| @@ -2333,6 +2333,7 @@ Isolate::Isolate(bool enable_serializer) | |
| random_number_generator_(NULL), | |
| rail_mode_(PERFORMANCE_ANIMATION), | |
| promise_hook_or_debug_is_active_(false), | |
| + atomics_wait_callback_data_(NULL), | |
| promise_hook_(NULL), | |
| load_start_time_ms_(0), | |
| serializer_enabled_(enable_serializer), | |
| @@ -3382,6 +3383,30 @@ void Isolate::SetHostImportModuleDynamicallyCallback( | |
| host_import_module_dynamically_callback_ = callback; | |
| } | |
| +void Isolate::SetAtomicsWaitCallback(v8::Isolate::AtomicsWaitCallback callback, | |
| + void* data) { | |
| + atomics_wait_callback_ = callback; | |
| + atomics_wait_callback_data_ = data; | |
| +} | |
| + | |
| +void Isolate::RunAtomicsWaitCallback(v8::Isolate::AtomicsWaitEvent event, | |
| + Handle<JSArrayBuffer> array_buffer, | |
| + size_t offset_in_bytes, | |
| + int32_t value, | |
| + double timeout_in_ms, | |
| + bool* waiting) { | |
| + DCHECK(array_buffer->is_shared()); | |
| + if (atomics_wait_callback_ == nullptr) | |
| + return; | |
| + atomics_wait_callback_(event, | |
| + v8::Utils::ToLocalShared(array_buffer), | |
| + offset_in_bytes, | |
| + value, | |
| + timeout_in_ms, | |
| + waiting, | |
| + atomics_wait_callback_data_); | |
| +} | |
| + | |
| void Isolate::SetPromiseHook(PromiseHook hook) { | |
| promise_hook_ = hook; | |
| DebugStateUpdated(); | |
| diff --git a/deps/v8/src/isolate.h b/deps/v8/src/isolate.h | |
| index a22bddf6bddf..d3a670fa73cb 100644 | |
| --- a/deps/v8/src/isolate.h | |
| +++ b/deps/v8/src/isolate.h | |
| @@ -1190,6 +1190,15 @@ class Isolate { | |
| void DebugStateUpdated(); | |
| + void SetAtomicsWaitCallback(v8::Isolate::AtomicsWaitCallback callback, | |
| + void* data); | |
| + void RunAtomicsWaitCallback(v8::Isolate::AtomicsWaitEvent event, | |
| + Handle<JSArrayBuffer> array_buffer, | |
| + size_t offset_in_bytes, | |
| + int32_t value, | |
| + double timeout_in_ms, | |
| + bool* waiting); | |
| + | |
| void SetPromiseHook(PromiseHook hook); | |
| void RunPromiseHook(PromiseHookType type, Handle<JSPromise> promise, | |
| Handle<Object> parent); | |
| @@ -1478,6 +1487,8 @@ class Isolate { | |
| base::RandomNumberGenerator* random_number_generator_; | |
| base::AtomicValue<RAILMode> rail_mode_; | |
| bool promise_hook_or_debug_is_active_; | |
| + v8::Isolate::AtomicsWaitCallback atomics_wait_callback_; | |
| + void* atomics_wait_callback_data_; | |
| PromiseHook promise_hook_; | |
| HostImportModuleDynamicallyCallback host_import_module_dynamically_callback_; | |
| base::Mutex rail_mutex_; | |
| diff --git a/src/env.cc b/src/env.cc | |
| index f0811396ca2a..7eafdd014950 100644 | |
| --- a/src/env.cc | |
| +++ b/src/env.cc | |
| @@ -4,6 +4,7 @@ | |
| #include "req-wrap-inl.h" | |
| #include "node_platform.h" | |
| #include "node_worker.h" | |
| +#include "node_messaging.h" | |
| #if defined(_MSC_VER) | |
| #define getpid GetCurrentProcessId | |
| @@ -70,6 +71,8 @@ IsolateData::IsolateData(Isolate* isolate, | |
| platform_(platform) { | |
| if (platform_ != nullptr) | |
| platform_->RegisterIsolate(this, event_loop); | |
| + isolate->SetAtomicsWaitCallback(AtomicsWaitCallback, | |
| + static_cast<void*>(isolate)); | |
| } | |
| IsolateData::~IsolateData() { | |
| diff --git a/src/node_messaging.cc b/src/node_messaging.cc | |
| index 058abb7bbf51..48e945c4e763 100644 | |
| --- a/src/node_messaging.cc | |
| +++ b/src/node_messaging.cc | |
| @@ -2,10 +2,12 @@ | |
| #include "node_contextify.h" | |
| #include "node_internals.h" | |
| #include "node_buffer.h" | |
| +#include "node_mutex.h" | |
| #include "util.h" | |
| #include "util-inl.h" | |
| #include "async-wrap.h" | |
| #include "async-wrap-inl.h" | |
| +#include <algorithm> | |
| using v8::Array; | |
| using v8::ArrayBuffer; | |
| @@ -35,6 +37,7 @@ using v8::ValueDeserializer; | |
| using v8::ValueSerializer; | |
| namespace node { | |
| + | |
| namespace worker { | |
| namespace { | |
| @@ -72,6 +75,35 @@ class SABLifetimePartner : public BaseObject { | |
| : BaseObject(env, obj), | |
| reference(r) { | |
| MakeWeak<SABLifetimePartner>(this); | |
| + | |
| + { | |
| + Mutex::ScopedLock lock(reference->accessing_isolates_mutex); | |
| + reference->accessing_isolates.insert(env->isolate()); | |
| + } | |
| + } | |
| + | |
| + ~SABLifetimePartner() { | |
| + bool all_isolates_are_waiting = true; | |
| + { | |
| + Mutex::ScopedLock lock(reference->accessing_isolates_mutex); | |
| + reference->accessing_isolates.erase( | |
| + reference->accessing_isolates.find(env()->isolate())); | |
| + for (Isolate* isolate : reference->accessing_isolates) { | |
| + if (reference->waiting_threads.count(isolate) == 0) { | |
| + all_isolates_are_waiting = false; | |
| + break; | |
| + } | |
| + } | |
| + } | |
| + | |
| + if (all_isolates_are_waiting) { | |
| + for (auto& info : reference->waiting_threads) { | |
| + info.second.cancelled = true; | |
| + info.first->RequestInterrupt([](Isolate* isolate, void* ptr){ | |
| + *static_cast<bool*>(ptr) = false; | |
| + }, static_cast<void*>(info.second.waiting)); | |
| + } | |
| + } | |
| } | |
| ExternalSABReference reference; | |
| @@ -80,7 +112,8 @@ class SABLifetimePartner : public BaseObject { | |
| } // anonymous namespace | |
| ExternalSABReference ReferenceCountedSAB::ForIncomingSharedArrayBuffer( | |
| - Environment* env, Local<Context> context, Local<SharedArrayBuffer> source) { | |
| + Environment* env, Local<Context> context, Local<SharedArrayBuffer> source, | |
| + bool may_attach_new_reference) { | |
| Local<Value> lifetime_partner; | |
| if (!source->GetPrivate(context, | |
| @@ -93,6 +126,8 @@ ExternalSABReference ReferenceCountedSAB::ForIncomingSharedArrayBuffer( | |
| env->sab_lifetimepartner_constructor_template() | |
| ->HasInstance(lifetime_partner)) { | |
| if (!source->IsExternal()) { | |
| + if (!may_attach_new_reference) | |
| + return nullptr; | |
| env->ThrowError("Found internalized SharedArrayBuffer with " | |
| "lifetime partner object"); | |
| return nullptr; | |
| @@ -104,6 +139,9 @@ ExternalSABReference ReferenceCountedSAB::ForIncomingSharedArrayBuffer( | |
| return partner->reference; | |
| } | |
| + if (!may_attach_new_reference) | |
| + return nullptr; | |
| + | |
| if (source->IsExternal()) { | |
| // If this is an external SharedArrayBuffer but we do not see a lifetime | |
| // partner object, we did not externalize it. In that case, there is no | |
| @@ -859,8 +897,61 @@ static void InitMessaging(Local<Object> target, | |
| } | |
| } // anonymous namespace | |
| - | |
| } // namespace worker | |
| + | |
| +void AtomicsWaitCallback(enum v8::Isolate::AtomicsWaitEvent event, | |
| + Local<SharedArrayBuffer> array_buffer, | |
| + size_t offset_in_bytes, | |
| + int32_t value, | |
| + double timeout_in_ms, | |
| + bool* waiting, | |
| + void* data) { | |
| + Isolate* isolate = static_cast<Isolate*>(data); | |
| + Local<Context> context = isolate->GetCurrentContext(); | |
| + Environment* env = Environment::GetCurrent(context); | |
| + worker::ExternalSABReference ref; | |
| + if (timeout_in_ms >= 0) | |
| + return; | |
| + if (event == Isolate::kAtomicsStartWait) { | |
| + ref = worker::ReferenceCountedSAB::ForIncomingSharedArrayBuffer( | |
| + env, context, array_buffer, false); | |
| + bool all_isolates_are_waiting = true; | |
| + if (ref != nullptr) { | |
| + Mutex::ScopedLock lock(ref->accessing_isolates_mutex); | |
| + for (Isolate* other_isolate : ref->accessing_isolates) { | |
| + if (isolate == other_isolate) | |
| + continue; | |
| + if (ref->waiting_threads.count(other_isolate) == 0) { | |
| + all_isolates_are_waiting = false; | |
| + break; | |
| + } | |
| + } | |
| + } | |
| + | |
| + if (all_isolates_are_waiting) { | |
| + env->ThrowError("Atomics.wait is unwakeable"); | |
| + return; | |
| + } | |
| + | |
| + if (ref == nullptr) { | |
| + ref = worker::ReferenceCountedSAB::ForIncomingSharedArrayBuffer( | |
| + env, context, array_buffer, true); | |
| + } | |
| + | |
| + ref->waiting_threads[isolate] = {waiting, false}; | |
| + } else { | |
| + ref = worker::ReferenceCountedSAB::ForIncomingSharedArrayBuffer( | |
| + env, env->context(), array_buffer, false); | |
| + bool cancelled = ref->waiting_threads[isolate].cancelled; | |
| + ref->waiting_threads.erase(isolate); | |
| + if (ref == nullptr) | |
| + return; | |
| + if (cancelled) { | |
| + env->ThrowError("Atomics.wait is unwakeable"); | |
| + } | |
| + } | |
| +} | |
| + | |
| } // namespace node | |
| NODE_MODULE_CONTEXT_AWARE_BUILTIN(messaging, node::worker::InitMessaging) | |
| diff --git a/src/node_messaging.h b/src/node_messaging.h | |
| index 9a64f9164de0..262b55659f06 100644 | |
| --- a/src/node_messaging.h | |
| +++ b/src/node_messaging.h | |
| @@ -5,6 +5,7 @@ | |
| #include "env.h" | |
| #include "node_mutex.h" | |
| +#include <set> | |
| #include <list> | |
| #include <memory> | |
| @@ -26,6 +27,11 @@ class MessagePort; | |
| // Generic offset for use by the JS core modules | |
| #define MESSAGE_FLAG_CUSTOM_OFFSET 100 | |
| +struct WaitingThreadInfo { | |
| + bool* waiting; | |
| + bool cancelled; | |
| +}; | |
| + | |
| // Any further flagged message codes are defined by the modules that use them. | |
| // This is an object associated with a SharedArrayBuffer, which keeps track | |
| @@ -42,7 +48,8 @@ class ReferenceCountedSAB | |
| public: | |
| static ExternalSABReference ForIncomingSharedArrayBuffer( | |
| Environment* env, v8::Local<v8::Context> context, | |
| - v8::Local<v8::SharedArrayBuffer> source); | |
| + v8::Local<v8::SharedArrayBuffer> source, | |
| + bool may_attach_new_reference = true); | |
| ~ReferenceCountedSAB(); | |
| // Create a SharedArrayBuffer object for a specific Environment and Context. | |
| @@ -57,6 +64,9 @@ class ReferenceCountedSAB | |
| ReferenceCountedSAB& operator=(const ReferenceCountedSAB&) = delete; | |
| ReferenceCountedSAB(const ReferenceCountedSAB&) = delete; | |
| + std::map<v8::Isolate*,WaitingThreadInfo> waiting_threads; | |
| + Mutex accessing_isolates_mutex; | |
| + std::multiset<v8::Isolate*> accessing_isolates; | |
| private: | |
| explicit ReferenceCountedSAB(void* data, size_t size); | |
| @@ -244,6 +254,15 @@ v8::MaybeLocal<v8::Function> GetMessagePortConstructor( | |
| Environment* env, v8::Local<v8::Context> context); | |
| } // namespace worker | |
| + | |
| +void AtomicsWaitCallback(enum v8::Isolate::AtomicsWaitEvent event, | |
| + v8::Local<v8::SharedArrayBuffer> array_buffer, | |
| + size_t offset_in_bytes, | |
| + int32_t value, | |
| + double timeout_in_ms, | |
| + bool* waiting, | |
| + void* data); | |
| + | |
| } // namespace node | |
| #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS | |
| diff --git a/test.js b/test.js | |
| new file mode 100644 | |
| index 000000000000..a5ae85d624f1 | |
| --- /dev/null | |
| +++ b/test.js | |
| @@ -0,0 +1,69 @@ | |
| +'use strict'; | |
| +// Flags: --harmony-sharedarraybuffer --expose-gc | |
| +const common = require('./test/common'); | |
| +const assert = require('assert'); | |
| +const worker = require('worker'); | |
| + | |
| +{ | |
| + const a = new SharedArrayBuffer(4); | |
| + const b = new Int32Array(a); | |
| + assert.throws(() => Atomics.wait(b, 0, 0), | |
| + /^Error: Atomics.wait is unwakeable$/); | |
| +} | |
| + | |
| +{ | |
| + const a = new SharedArrayBuffer(4); | |
| + const w = new worker.Worker(` | |
| + const worker = require('worker'); | |
| + worker.once('workerMessage', (msg) => { | |
| + worker.postMessage(msg); | |
| + msg = null; | |
| + setImmediate(gc); | |
| + }); | |
| + `, { eval: true }); | |
| + w.postMessage(a); | |
| + w.on('message', common.mustCall((a) => { | |
| + const b = new Int32Array(a); | |
| + assert.throws(() => Atomics.wait(b, 0, 0), | |
| + /^Error: Atomics.wait is unwakeable$/); | |
| + })); | |
| +} | |
| + | |
| +{ | |
| + const a = new SharedArrayBuffer(8); | |
| + const b = new Int32Array(a); | |
| + b[1] = 12345; | |
| + const w = new worker.Worker(` | |
| + const worker = require('worker'); | |
| + const assert = require('assert'); | |
| + worker.once('workerMessage', (b) => { | |
| + assert.strictEqual(b[1], 12345); | |
| + worker.postMessage('sleeping'); | |
| + Atomics.wait(b, 0, 0); | |
| + assert.strictEqual(b[1], 54321); | |
| + }); | |
| + `, { eval: true }); | |
| + w.postMessage(b); | |
| + w.on('exit', common.mustCall()); | |
| + w.on('message', common.mustCall((msg) => { | |
| + assert.strictEqual(msg, 'sleeping'); | |
| + b[0] = 1; | |
| + b[1] = 54321; | |
| + Atomics.wake(b, 0); | |
| + })); | |
| +} | |
| + | |
| +{ | |
| + let a = new SharedArrayBuffer(4); | |
| + const w = new worker.Worker(` | |
| + const worker = require('worker'); | |
| + const assert = require('assert'); | |
| + worker.once('workerMessage', (a) => { | |
| + const b = new Int32Array(a); | |
| + assert.throws(() => Atomics.wait(b, 0, 0), | |
| + /^Error: Atomics.wait is unwakeable$/); | |
| + }); | |
| + `, { eval: true }); | |
| + w.postMessage(a); | |
| + setImmediate(gc); | |
| +} |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment