Instantly share code, notes, and snippets.

@addaleax /poc.diff Secret
Created Oct 7, 2017

Embed
What would you like to do?
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