Jakob had suggested that I post a proposal to v8-users related to a thread
cleanup issue <https://groups.google.com/forum/#!topic/v8-users/iZ1gz1KThgs>
which I plan on doing, but his comments about threads going in and out of
isolates got me curious about thread entry/exit mechanics which led me to
puzzlement about the use of Isolate::entry_stack_. Specifically, it seemed
to make no sense at all to me for this to be in Isolate. The entry stack is
a per thread thing and no one other than the thread that owns a stack entry
needs to look at it so why is it in Isolate? It wouldn't matter if this
were harmless but it seems that Isolate::Enter happily sets a pointer to
another thread's EntryStackItem if one happened to be in entry_stack_ for
the new Isolate and Isolate:Exit then happily sets it back to whatever it
was when it entered the isolate, regardless of what that thread or other
threads that might have entered this Isolate are doing. Having one thread
messing with another's stack entries seems clearly wrong since obviously
one point of threads is to allow independent stacks for the threads.
I've attached a hacked version of hello-world (sorry, it probably only runs
on linux and maybe some unixes) that demonstrates a bug check that goes off
in Isolate::Exit as a result of this problem. The basic sequence is that
there are two threads and two isolates:
1. Thread 0 enters isolate 1 - Sets entry_stack_ for that isolate to say
A0
2. Thread 1 enters isolate 1 - Sets entry_stack_ for that isolate to
say A1 - A1->previous_item_ == A0.
3. Thread 0 enters isolate 2 - Sets entry_stack_ for that isolate to say
B0
4. Thread 1 enters isolate 2 - Sets entry_stack_ for that isolate to say
B1 - B1->previous_item_ == B0
5. Thread 0 exits isolate 2 - It finds entry_stack_ pointing to B1 and
then finds B1->previous_thread_data pointing to thread 1's data and the bug
check goes off. End of story.
The bug check that goes off is in Isolate::Exit
DCHECK(entry_stack_->previous_thread_data == NULL ||
entry_stack_->previous_thread_data->thread_id().Equals(
ThreadId::Current()));
Obviously this only goes off for a debug build of the attached code. I
imagine one could produce a spectacular mess with a release version if one
used more isolates and threads but I think the debug build is sufficient to
prove the point.
While this could be fixed by thread data archive and restore, archiving and
restoring entry_stack_ this would seem silly to me as again, there seems
nothing particularly useful about having entry_stack_ in Isolate. It would
seem that the best approach would be to somehow combine EntryStackItem with
PerIsolateThreadData or at least set the latter to point at the former, but
since I might be wrong about all this and would need to do quite a bit more
research even if I weren't, I won't float a proposal here.
Maybe I should have posted this to v8-users? But this seems kinda
internalsy to me so v8-dev seemed right.
Thanks
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups
"v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
// Copyright 2015 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <semaphore.h>
#include <thread>
#include "include/libplatform/libplatform.h"
#include "include/v8.h"
using namespace v8;
class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
public:
virtual void* Allocate(size_t length) {
void* data = AllocateUninitialized(length);
return data == NULL ? data : memset(data, 0, length);
}
virtual void* AllocateUninitialized(size_t length) { return malloc(length); }
virtual void Free(void* data, size_t) { free(data); }
};
class Silly {
public:
Silly(Isolate* isolate1, Isolate* isolate2,
sem_t* semaphores, int32_t number) :
isolate1(isolate1),
isolate2(isolate2),
semaphores(semaphores),
number(number) {
}
Isolate* isolate1;
Isolate* isolate2;
sem_t* semaphores;
int32_t number;
};
#define NTHREADS 2
// One extra to not be able to not check for the last post
#define NSEMAPHORES NTHREADS * 3 + 1
static void sayHello(Silly* silly, Isolate* isolate) {
// Create a stack-allocated handle scope.
HandleScope handle_scope(isolate);
// Create a new context.
Local<Context> context = Context::New(isolate);
// Enter the context for compiling and running the hello world script.
Context::Scope context_scope(context);
Local<Object> global = context->Global();
#define UTF8STRING(VALUE) String::NewFromUtf8(isolate, #VALUE, \
NewStringType::kNormal).ToLocalChecked()
global->Set(context, UTF8STRING(number),
Integer::New(isolate, silly->number)).FromJust();
// Create a string containing the JavaScript source code.
Local<String> source = UTF8STRING('Hello' + ', World! ' + number);
#undef UTF8STRING
// Compile the source code.
Local<Script> script = Script::Compile(context, source).ToLocalChecked();
// Run the script to get the result.
Local<Value> result = script->Run(context).ToLocalChecked();
// Convert the result to an UTF8 string and print it.
String::Utf8Value utf8(result);
printf("%s\n", *utf8);
}
static void goofyThread(Silly* silly) {
// Ensure deterministic isolate entry order
sem_wait(&silly->semaphores[silly->number]);
Isolate* isolate1 = silly->isolate1;
Locker locker(isolate1);
Isolate::Scope isolate_scope(isolate1);
sem_post(&silly->semaphores[silly->number + 1]);
sayHello(silly, isolate1);
{
Unlocker unlocker(isolate1);
{
// Ensure deterministic isolate entry order
sem_wait(&silly->semaphores[NTHREADS + silly->number]);
Isolate* isolate2 = silly->isolate2;
Locker locker(isolate2);
Isolate::Scope isolate_scope(isolate2);
sem_post(&silly->semaphores[NTHREADS + silly->number + 1]);
sayHello(silly, isolate2);
{
Unlocker unlocker(isolate2);
// Ensure deterministic isolate exit order
sem_wait(&silly->semaphores[NTHREADS * 2 + silly->number]);
}
sem_post(&silly->semaphores[NTHREADS * 2 + silly->number + 1]);
}
}
}
int main(int argc, char* argv[]) {
// Initialize V8.
V8::InitializeICU();
V8::InitializeExternalStartupData(argv[0]);
Platform* platform = platform::CreateDefaultPlatform();
V8::InitializePlatform(platform);
V8::Initialize();
// Create two new Isolates and make the first the current one.
ArrayBufferAllocator allocator;
Isolate::CreateParams create_params;
create_params.array_buffer_allocator = &allocator;
Isolate* isolate1 = Isolate::New(create_params);
Isolate* isolate2 = Isolate::New(create_params);
sem_t semaphores[NSEMAPHORES];
for (int i = 0; i < NSEMAPHORES; i++) {
sem_init(&semaphores[i], 0, 0);
}
Silly* sillies[NTHREADS];
std::thread threads[NTHREADS];
for (int i = 0; i < NTHREADS; i++) {
sillies[i] = new Silly(isolate1, isolate2, semaphores, i);
threads[i] = std::thread(goofyThread, sillies[i]);
}
// Let's get this show on the road
sem_post(&semaphores[0]);
for (int i = 0; i < NTHREADS; i++) threads[i].join();
for (int i = 0; i < NSEMAPHORES; i++)
sem_destroy(&semaphores[i]);
// Dispose the isolates and tear down V8.
isolate1->Dispose();
isolate2->Dispose();
for (int i = 0; i < NTHREADS; i++) delete sillies[i];
V8::Dispose();
V8::ShutdownPlatform();
delete platform;
return 0;
}
