diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc index 2746e89fb8..e26d608138 100644 --- a/src/execution/isolate.cc +++ b/src/execution/isolate.cc @@ -3069,21 +3069,23 @@ void Isolate::AddSharedWasmMemory(Handle memory_object) { void Isolate::RecordStackSwitchForScanning() { Object current = root(RootIndex::kActiveContinuation); DCHECK(!current.IsUndefined()); - thread_local_top()->stack_.ClearStackSegments(); - wasm::StackMemory* stack = Managed::cast( - WasmContinuationObject::cast(current).stack()) - .get() - .get(); + stack().ClearStackSegments(); + wasm::StackMemory* wasm_stack = + Managed::cast( + WasmContinuationObject::cast(current).stack()) + .get() + .get(); current = WasmContinuationObject::cast(current).parent(); - heap()->SetStackStart(reinterpret_cast(stack->base())); + heap()->SetStackStart(reinterpret_cast(wasm_stack->base())); // We don't need to add all inactive stacks. Only the ones in the active chain // may contain cpp heap pointers. while (!current.IsUndefined()) { auto cont = WasmContinuationObject::cast(current); - auto* stack = Managed::cast(cont.stack()).get().get(); - thread_local_top()->stack_.AddStackSegment( - reinterpret_cast(stack->base()), - reinterpret_cast(stack->jmpbuf()->sp)); + auto* wasm_stack = + Managed::cast(cont.stack()).get().get(); + stack().AddStackSegment( + reinterpret_cast(wasm_stack->base()), + reinterpret_cast(wasm_stack->jmpbuf()->sp)); current = cont.parent(); } } @@ -3371,23 +3373,13 @@ void Isolate::Delete(Isolate* isolate) { Isolate* saved_isolate = isolate->TryGetCurrent(); SetIsolateThreadLocals(isolate, nullptr); isolate->set_thread_id(ThreadId::Current()); - if (saved_isolate) { - isolate->thread_local_top()->stack_ = - std::move(saved_isolate->thread_local_top()->stack_); - } else { - isolate->heap()->SetStackStart(base::Stack::GetStackStart()); - } + isolate->heap()->SetStackStart(base::Stack::GetStackStart()); bool owns_shared_isolate = isolate->owns_shared_isolate_; Isolate* maybe_shared_isolate = isolate->shared_isolate_; isolate->Deinit(); - // Restore the saved isolate's stack. - if (saved_isolate) - saved_isolate->thread_local_top()->stack_ = - std::move(isolate->thread_local_top()->stack_); - #ifdef DEBUG non_disposed_isolates_--; #endif // DEBUG @@ -4618,6 +4610,10 @@ bool Isolate::Init(SnapshotData* startup_snapshot_data, void Isolate::Enter() { Isolate* current_isolate = nullptr; PerIsolateThreadData* current_data = CurrentPerIsolateThreadData(); + + // Set the stack start for the main thread that enters the isolate. + heap()->SetStackStart(base::Stack::GetStackStart()); + if (current_data != nullptr) { current_isolate = current_data->isolate_; DCHECK_NOT_NULL(current_isolate); diff --git a/src/execution/isolate.h b/src/execution/isolate.h index 8376116629..63cc4c79ef 100644 --- a/src/execution/isolate.h +++ b/src/execution/isolate.h @@ -32,6 +32,7 @@ #include "src/execution/stack-guard.h" #include "src/handles/handles.h" #include "src/handles/traced-handles.h" +#include "src/heap/base/stack.h" #include "src/heap/factory.h" #include "src/heap/heap.h" #include "src/heap/read-only-heap.h" @@ -2029,6 +2030,8 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory { SimulatorData* simulator_data() { return simulator_data_; } #endif + ::heap::base::Stack& stack() { return stack_; } + #ifdef V8_ENABLE_WEBASSEMBLY wasm::StackMemory*& wasm_stacks() { return wasm_stacks_; } // Update the thread local's Stack object so that it is aware of the new stack @@ -2527,6 +2530,9 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory { // The mutex only guards adding pages, the retrieval is signal safe. base::Mutex code_pages_mutex_; + // Stack information for the main thread. + ::heap::base::Stack stack_; + #ifdef V8_ENABLE_WEBASSEMBLY wasm::StackMemory* wasm_stacks_; #endif diff --git a/src/execution/thread-local-top.cc b/src/execution/thread-local-top.cc index c115ae04bb..05cc20b8e4 100644 --- a/src/execution/thread-local-top.cc +++ b/src/execution/thread-local-top.cc @@ -37,7 +37,6 @@ void ThreadLocalTop::Clear() { current_embedder_state_ = nullptr; failed_access_check_callback_ = nullptr; thread_in_wasm_flag_address_ = kNullAddress; - stack_ = ::heap::base::Stack(); } void ThreadLocalTop::Initialize(Isolate* isolate) { @@ -45,12 +44,8 @@ void ThreadLocalTop::Initialize(Isolate* isolate) { isolate_ = isolate; thread_id_ = ThreadId::Current(); #if V8_ENABLE_WEBASSEMBLY - stack_.SetStackStart(base::Stack::GetStackStart(), - v8_flags.experimental_wasm_stack_switching); thread_in_wasm_flag_address_ = reinterpret_cast
( trap_handler::GetThreadInWasmThreadLocalAddress()); -#else - stack_.SetStackStart(base::Stack::GetStackStart(), false); #endif // V8_ENABLE_WEBASSEMBLY #ifdef USE_SIMULATOR simulator_ = Simulator::current(isolate); diff --git a/src/execution/thread-local-top.h b/src/execution/thread-local-top.h index 43fec0a7df..989c817f31 100644 --- a/src/execution/thread-local-top.h +++ b/src/execution/thread-local-top.h @@ -10,7 +10,6 @@ #include "include/v8-unwinder.h" #include "src/common/globals.h" #include "src/execution/thread-id.h" -#include "src/heap/base/stack.h" #include "src/objects/contexts.h" #include "src/utils/utils.h" @@ -30,7 +29,7 @@ class ThreadLocalTop { // TODO(all): This is not particularly beautiful. We should probably // refactor this to really consist of just Addresses and 32-bit // integer fields. - static constexpr uint32_t kSizeInBytes = 30 * kSystemPointerSize; + static constexpr uint32_t kSizeInBytes = 25 * kSystemPointerSize; // Does early low-level initialization that does not depend on the // isolate being present. @@ -147,9 +146,6 @@ class ThreadLocalTop { // Address of the thread-local "thread in wasm" flag. Address thread_in_wasm_flag_address_; - - // Stack information. - ::heap::base::Stack stack_; }; } // namespace internal diff --git a/src/heap/heap.cc b/src/heap/heap.cc index bc2c07d99d..e48b29400b 100644 --- a/src/heap/heap.cc +++ b/src/heap/heap.cc @@ -5813,9 +5813,7 @@ void Heap::SetStackStart(void* stack_start) { #endif // V8_ENABLE_WEBASSEMBLY } -::heap::base::Stack& Heap::stack() { - return isolate_->thread_local_top()->stack_; -} +::heap::base::Stack& Heap::stack() { return isolate_->stack(); } void Heap::StartTearDown() { // Finish any ongoing sweeping to avoid stray background tasks still accessing diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 4f17f227fa..2ade47697a 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -13043,12 +13043,13 @@ bool ApiTestFuzzer::NextThread() { void ApiTestFuzzer::Run() { - // When it is our turn... + // Wait until it is our turn. gate_.Wait(); { - // ... get the V8 lock + // Get the V8 lock. v8::Locker locker(CcTest::isolate()); - // ... and start running the test. + // Start running the test, which will enter the isolate and exit it when it + // finishes. CallTest(); } // This test finished. @@ -13089,11 +13090,18 @@ static void CallTestNumber(int test_number) { void ApiTestFuzzer::RunAllTests() { + // This method is called when running each THREADING_TEST, which is an + // initialized test and has entered the isolate at this point. We need to exit + // the isolate, so that the fuzzer threads can enter it in turn, while running + // their tests. + CcTest::isolate()->Exit(); // Set off the first test. current_ = -1; NextThread(); // Wait till they are all done. all_tests_done_.Wait(); + // We enter the isolate again, to prepare for teardown. + CcTest::isolate()->Enter(); } @@ -13111,10 +13119,16 @@ int ApiTestFuzzer::GetNextTestNumber() { void ApiTestFuzzer::ContextSwitch() { // If the new thread is the same as the current thread there is nothing to do. if (NextThread()) { - // Now it can start. - v8::Unlocker unlocker(CcTest::isolate()); - // Wait till someone starts us again. - gate_.Wait(); + // Exit the isolate from this thread. + CcTest::i_isolate()->Exit(); + { + // Now the new thread can start. + v8::Unlocker unlocker(CcTest::isolate()); + // Wait till someone starts us again. + gate_.Wait(); + } + // Enter the isolate from this thread again. + CcTest::i_isolate()->Enter(); // And we're off. } } diff --git a/test/cctest/test-cpu-profiler.cc b/test/cctest/test-cpu-profiler.cc index 59cef18d78..5b3d08d79b 100644 --- a/test/cctest/test-cpu-profiler.cc +++ b/test/cctest/test-cpu-profiler.cc @@ -3457,8 +3457,12 @@ TEST(MultipleThreadsSingleIsolate) { env, "YieldIsolate", [](const v8::FunctionCallbackInfo& info) { v8::Isolate* isolate = info.GetIsolate(); if (!info[0]->IsTrue()) return; - v8::Unlocker unlocker(isolate); - v8::base::OS::Sleep(v8::base::TimeDelta::FromMilliseconds(1)); + isolate->Exit(); + { + v8::Unlocker unlocker(isolate); + v8::base::OS::Sleep(v8::base::TimeDelta::FromMilliseconds(1)); + } + isolate->Enter(); }); CompileRun(varying_frame_size_script); @@ -3470,11 +3474,13 @@ TEST(MultipleThreadsSingleIsolate) { // For good measure, profile on our own thread UnlockingThread::Profile(env, 0); + isolate->Exit(); { v8::Unlocker unlocker(isolate); thread1.Join(); thread2.Join(); } + isolate->Enter(); } // Tests that StopProfiling doesn't wait for the next sample tick in order to diff --git a/test/cctest/test-debug.cc b/test/cctest/test-debug.cc index 06e0849779..7b39bb3530 100644 --- a/test/cctest/test-debug.cc +++ b/test/cctest/test-debug.cc @@ -4211,6 +4211,12 @@ class ArchiveRestoreThread : public v8::base::Thread, // child.GetBreakCount() will return 1 if the debugger fails to stop // on the `next()` line after the grandchild thread returns. CHECK_EQ(child.GetBreakCount(), 5); + + // This test on purpose unlocks the isolate without exiting and + // re-entering. It must however update the stack start, which would have + // been done automatically if the isolate was properly re-entered. + reinterpret_cast(isolate_)->heap()->SetStackStart( + v8::base::Stack::GetStackStart()); } } diff --git a/test/cctest/test-lockers.cc b/test/cctest/test-lockers.cc index c33c94a818..b75e68bc0a 100644 --- a/test/cctest/test-lockers.cc +++ b/test/cctest/test-lockers.cc @@ -871,7 +871,9 @@ TEST(LockUnlockLockDefaultIsolateMultithreaded) { threads.push_back(new LockUnlockLockDefaultIsolateThread(context)); } } + CcTest::isolate()->Exit(); StartJoinAndDeleteThreads(threads); + CcTest::isolate()->Enter(); }