Fix return type of IsolateEnvironment::GetCurrent()

This pointer is always dereferenced and therefore should be a reference.

src/external_copy/external_copy.cc

@@ -164,7 +164,7 @@ auto ExternalCopy::Copy(Local<Value> value, bool transfer_out, ArrayRange transf
 		// `Buffer()` below will force v8 to attempt to create a buffer if it doesn't exist, and if
 		// there is an allocation failure it will crash the process.
 		if (!view->HasBuffer()) {
-			auto* allocator = IsolateEnvironment::GetCurrent()->GetLimitedAllocator();
+			auto* allocator = IsolateEnvironment::GetCurrent().GetLimitedAllocator();
 			if (allocator != nullptr && !allocator->Check(view->ByteLength())) {
 				throw RuntimeRangeError("Array buffer allocation failed");
 			}
@@ -292,7 +292,7 @@ auto ExternalCopy::CopyThrownValue(Local<Value> value) -> std::unique_ptr<Extern
 }
 
 auto ExternalCopy::CopyIntoCheckHeap(bool transfer_in) -> Local<Value> {
-	IsolateEnvironment::HeapCheck heap_check{*IsolateEnvironment::GetCurrent()};
+	IsolateEnvironment::HeapCheck heap_check{IsolateEnvironment::GetCurrent()};
 	auto value = CopyInto(transfer_in);
 	heap_check.Epilogue();
 	return value;
@@ -412,13 +412,13 @@ auto ExternalCopyArrayBuffer::CopyInto(bool transfer_in) -> Local<Value> {
 		UpdateSize(0);
 		size_t size = backing_store->ByteLength();
 		auto handle = ArrayBuffer::New(Isolate::GetCurrent(), std::move(backing_store));
-		auto* allocator = IsolateEnvironment::GetCurrent()->GetLimitedAllocator();
+		auto* allocator = IsolateEnvironment::GetCurrent().GetLimitedAllocator();
 		if (allocator != nullptr) {
 			allocator->Track(handle, size);
 		}
 		return handle;
 	} else {
-		auto* allocator = IsolateEnvironment::GetCurrent()->GetLimitedAllocator();
+		auto* allocator = IsolateEnvironment::GetCurrent().GetLimitedAllocator();
 		auto backing_store = *this->backing_store.read();
 		if (!backing_store) {
 			throw RuntimeGenericError("Array buffer is invalid");
@@ -446,7 +446,7 @@ auto ExternalCopySharedArrayBuffer::CopyInto(bool /*transfer_in*/) -> Local<Valu
 	auto backing_store = *this->backing_store.read();
 	size_t size = backing_store->ByteLength();
 	auto handle = SharedArrayBuffer::New(Isolate::GetCurrent(), std::move(backing_store));
-	auto* allocator = IsolateEnvironment::GetCurrent()->GetLimitedAllocator();
+	auto* allocator = IsolateEnvironment::GetCurrent().GetLimitedAllocator();
 	if (allocator != nullptr) {
 		allocator->Track(handle, size);
 	}

src/external_copy/serializer.h

@@ -89,7 +89,7 @@ class BaseSerializer {
 			delegate.SetDeserializer(&deserializer);
 
 			// Watch for allocation errors
-			auto* allocator = IsolateEnvironment::GetCurrent()->GetLimitedAllocator();
+			auto* allocator = IsolateEnvironment::GetCurrent().GetLimitedAllocator();
 			int failures = allocator == nullptr ? 0 : allocator->GetFailureCount();
 			Unmaybe(deserializer.ReadHeader(context));
 

src/external_copy/string.cc

@@ -14,13 +14,13 @@ namespace {
 class ExternalString final : public v8::String::ExternalStringResource {
 	public:
 		explicit ExternalString(std::shared_ptr<std::vector<char>> value) : value{std::move(value)} {
-			IsolateEnvironment::GetCurrent()->AdjustExtraAllocatedMemory(this->value->size());
+			IsolateEnvironment::GetCurrent().AdjustExtraAllocatedMemory(this->value->size());
 		}
 
 		ExternalString(const ExternalString&) = delete;
 
 		~ExternalString() final {
-			IsolateEnvironment::GetCurrent()->AdjustExtraAllocatedMemory(-static_cast<int>(this->value->size()));
+			IsolateEnvironment::GetCurrent().AdjustExtraAllocatedMemory(-static_cast<int>(this->value->size()));
 		}
 
 		auto operator= (const ExternalString&) = delete;
@@ -40,13 +40,13 @@ class ExternalString final : public v8::String::ExternalStringResource {
 class ExternalStringOneByte final : public v8::String::ExternalOneByteStringResource {
 	public:
 		explicit ExternalStringOneByte(std::shared_ptr<std::vector<char>> value) : value{std::move(value)} {
-			IsolateEnvironment::GetCurrent()->AdjustExtraAllocatedMemory(this->value->size());
+			IsolateEnvironment::GetCurrent().AdjustExtraAllocatedMemory(this->value->size());
 		}
 
 		ExternalStringOneByte(const ExternalStringOneByte&) = delete;
 
 		~ExternalStringOneByte() final {
-			IsolateEnvironment::GetCurrent()->AdjustExtraAllocatedMemory(-static_cast<int>(this->value->size()));
+			IsolateEnvironment::GetCurrent().AdjustExtraAllocatedMemory(-static_cast<int>(this->value->size()));
 		}
 
 		auto operator= (const ExternalStringOneByte&) = delete;

src/isolate/allocator_nortti.cc

@@ -12,14 +12,14 @@ class ExternalMemoryHandle {
 		ExternalMemoryHandle(Local<Object> local_handle, size_t size) :
 				handle{Isolate::GetCurrent(), local_handle}, size{size} {
 			handle.SetWeak(reinterpret_cast<void*>(this), &WeakCallbackV8, WeakCallbackType::kParameter);
-			IsolateEnvironment::GetCurrent()->AddWeakCallback(&handle, WeakCallback, this);
+			IsolateEnvironment::GetCurrent().AddWeakCallback(&handle, WeakCallback, this);
 		}
 
 		ExternalMemoryHandle(const ExternalMemoryHandle&) = delete;
 		auto operator=(const ExternalMemoryHandle&) = delete;
 
 		~ExternalMemoryHandle() {
-			auto* allocator = IsolateEnvironment::GetCurrent()->GetLimitedAllocator();
+			auto* allocator = IsolateEnvironment::GetCurrent().GetLimitedAllocator();
 			if (allocator != nullptr) {
 				allocator->AdjustAllocatedSize(-size);
 			}
@@ -32,7 +32,7 @@ class ExternalMemoryHandle {
 
 		static void WeakCallback(void* param) {
 			auto* that = reinterpret_cast<ExternalMemoryHandle*>(param);
-			IsolateEnvironment::GetCurrent()->RemoveWeakCallback(&that->handle);
+			IsolateEnvironment::GetCurrent().RemoveWeakCallback(&that->handle);
 			that->handle.Reset();
 			delete that;
 		}

src/isolate/class_handle.h

@@ -116,8 +116,8 @@ class ClassHandle {
 		 */
 		template <typename P, void (*F)(P*)>
 		void SetWeak(P* param) {
-			auto* isolate = IsolateEnvironment::GetCurrent();
-			isolate->AddWeakCallback(&this->handle, (void(*)(void*))F, param);
+			auto& isolate = IsolateEnvironment::GetCurrent();
+			isolate.AddWeakCallback(&this->handle, (void(*)(void*))F, param);
 			handle.SetWeak(param, WeakCallback<P, F>, v8::WeakCallbackType::kParameter);
 		}
 		template <typename P, void (*F)(P*)>
@@ -129,9 +129,9 @@ class ClassHandle {
 		 * Invoked once JS loses all references to this object
 		 */
 		static void WeakCallback(void* param) {
-			auto* isolate = IsolateEnvironment::GetCurrent();
+			auto& isolate = IsolateEnvironment::GetCurrent();
 			auto* that = reinterpret_cast<ClassHandle*>(param);
-			isolate->RemoveWeakCallback(&that->handle);
+			isolate.RemoveWeakCallback(&that->handle);
 			delete that; // NOLINT
 		}
 

src/isolate/environment.cc

@@ -108,7 +108,7 @@ void IsolateEnvironment::HeapCheck::Epilogue() {
  * IsolateEnvironment implementation
  */
 void IsolateEnvironment::OOMErrorCallback(const char* location, bool is_heap_oom) {
-	if (RaiseCatastrophicError(IsolateEnvironment::GetCurrent()->error_handler, "Catastrophic out-of-memory error")) {
+	if (RaiseCatastrophicError(IsolateEnvironment::GetCurrent().error_handler, "Catastrophic out-of-memory error")) {
 		while (true) {
 			using namespace std::chrono_literals;
 			std::this_thread::sleep_for(100s);
@@ -142,15 +142,15 @@ void IsolateEnvironment::OOMErrorCallback(const char* location, bool is_heap_oom
 }
 
 void IsolateEnvironment::PromiseRejectCallback(PromiseRejectMessage rejection) {
-	auto* that = IsolateEnvironment::GetCurrent();
-	assert(that->isolate == Isolate::GetCurrent());
+	auto& that = IsolateEnvironment::GetCurrent();
+	assert(that.isolate == Isolate::GetCurrent());
 	// TODO: Revisit this in version 4.x?
 	auto event = rejection.GetEvent();
 	if (event == kPromiseRejectWithNoHandler) {
-		that->unhandled_promise_rejections.emplace_back(that->isolate, rejection.GetPromise());
-		that->unhandled_promise_rejections.back().SetWeak();
+		that.unhandled_promise_rejections.emplace_back(that.isolate, rejection.GetPromise());
+		that.unhandled_promise_rejections.back().SetWeak();
 	} else if (event == kPromiseHandlerAddedAfterReject) {
-		that->PromiseWasHandled(rejection.GetPromise());
+		that.PromiseWasHandled(rejection.GetPromise());
 	}
 }
 
@@ -163,11 +163,11 @@ void IsolateEnvironment::PromiseWasHandled(v8::Local<v8::Promise> promise) {
 }
 
 auto IsolateEnvironment::CodeGenCallback(Local<Context> /*context*/, Local<Value> source) -> ModifyCodeGenerationFromStringsResult {
-	auto* that = IsolateEnvironment::GetCurrent();
+	auto& that = IsolateEnvironment::GetCurrent();
 	// This heuristic could be improved by looking up how much `timeout` this isolate has left and
 	// returning early in some cases
 	ModifyCodeGenerationFromStringsResult result;
-	if (source->IsString() && static_cast<size_t>(source.As<String>()->Length()) > static_cast<size_t>(that->memory_limit / 8)) {
+	if (source->IsString() && static_cast<size_t>(source.As<String>()->Length()) > static_cast<size_t>(that.memory_limit / 8)) {
 		return result;
 	}
 	result.codegen_allowed = true;
@@ -585,7 +585,7 @@ void IsolateEnvironment::RemoveWeakCallback(Persistent<Value>* handle) {
 }
 
 void AdjustRemotes(int delta) {
-	IsolateEnvironment::GetCurrent()->AdjustRemotes(delta);
+	IsolateEnvironment::GetCurrent().AdjustRemotes(delta);
 }
 
 auto RaiseCatastrophicError(RemoteHandle<Function>& handler, const char* message) -> bool {

src/isolate/environment.h

@@ -197,8 +197,11 @@ class IsolateEnvironment {
 		/**
 		 * Return pointer the currently running IsolateEnvironment
 		 */
-		static auto GetCurrent() -> IsolateEnvironment* {
-			return Executor::GetCurrentEnvironment();
+		static auto GetCurrent() -> IsolateEnvironment& {
+			auto* environment = Executor::GetCurrentEnvironment();
+			assert(environment != nullptr);
+			return *environment;
+
 		}
 
 		/**

src/isolate/external.h

@@ -25,7 +25,7 @@ class ExternalHolder {
 			new(that) ExternalHolder(external, std::forward<Args>(args)...);
 			// Setup weak callbacks (technically could throw)
 			that->handle.SetWeak(reinterpret_cast<void*>(that), WeakCallbackV8, v8::WeakCallbackType::kParameter);
-			IsolateEnvironment::GetCurrent()->AddWeakCallback(&that->handle, WeakCallback, that);
+			IsolateEnvironment::GetCurrent().AddWeakCallback(&that->handle, WeakCallback, that);
 			// Return local external handle
 			return external;
 		}
@@ -37,8 +37,8 @@ class ExternalHolder {
 
 		static void WeakCallback(void* param) {
 			auto* that = static_cast<ExternalHolder*>(param);
-			auto* isolate = IsolateEnvironment::GetCurrent();
-			isolate->RemoveWeakCallback(&that->handle);
+			auto& isolate = IsolateEnvironment::GetCurrent();
+			isolate.RemoveWeakCallback(&that->handle);
 			that->handle.Reset();
 			delete that;
 		}

src/isolate/run_with_timeout.h

@@ -47,7 +47,7 @@ class TimeoutRunner final : public Runnable {
 				}
 			}();
 			if (will_terminate) {
-				auto& env = *IsolateEnvironment::GetCurrent();
+				auto& env = IsolateEnvironment::GetCurrent();
 				auto* isolate = env.GetIsolate();
 				state.stack_trace = StackTraceHolder::RenderSingleStack(v8::StackTrace::CurrentStackTrace(isolate, 10));
 				isolate->TerminateExecution();
@@ -64,7 +64,7 @@ class TimeoutRunner final : public Runnable {
  */
 template <typename F>
 auto RunWithTimeout(uint32_t timeout_ms, F&& fn) -> v8::Local<v8::Value> {
-	IsolateEnvironment& isolate = *IsolateEnvironment::GetCurrent();
+	IsolateEnvironment& isolate = IsolateEnvironment::GetCurrent();
 	thread_suspend_handle thread_suspend{};
 	bool is_default_thread = Executor::IsDefaultThread();
 	bool did_terminate = false;

src/isolate/three_phase_task.cc

@@ -15,21 +15,20 @@ ThreePhaseTask::CalleeInfo::CalleeInfo(
 	Local<Context> context,
 	Local<StackTrace> stack_trace
 ) : remotes(resolver, context, stack_trace) {
-	IsolateEnvironment* env = IsolateEnvironment::GetCurrent();
-	if (env->IsDefault()) {
-		async = node::EmitAsyncInit(env->GetIsolate(), resolver->GetPromise(), StringTable::Get().isolatedVm);
+	auto& env = IsolateEnvironment::GetCurrent();
+	if (env.IsDefault()) {
+		async = node::EmitAsyncInit(env.GetIsolate(), resolver->GetPromise(), StringTable::Get().isolatedVm);
 	}
 }
 
 ThreePhaseTask::CalleeInfo::CalleeInfo(CalleeInfo&& that) noexcept :
 		remotes{std::move(that.remotes)}, async{std::exchange(that.async, {0, 0})} {}
-	IsolateEnvironment* env = IsolateEnvironment::GetCurrent();
 
 ThreePhaseTask::CalleeInfo::~CalleeInfo() {
-	IsolateEnvironment* env = IsolateEnvironment::GetCurrent();
+	auto& env = IsolateEnvironment::GetCurrent();
 	node::async_context tmp{0, 0};
-	if (env->IsDefault() && std::memcmp(&async, &tmp, sizeof(node::async_context)) != 0) {
-		node::EmitAsyncDestroy(env->GetIsolate(), async);
+	if (env.IsDefault() && std::memcmp(&async, &tmp, sizeof(node::async_context)) != 0) {
+		node::EmitAsyncDestroy(env.GetIsolate(), async);
 	}
 }
 
@@ -44,9 +43,9 @@ struct CallbackScope {
 	unique_ptr<node::CallbackScope> scope;
 
 	CallbackScope(node::async_context async, Local<Object> resource) {
-		IsolateEnvironment* env = IsolateEnvironment::GetCurrent();
-		if (env->IsDefault()) {
-			scope = std::make_unique<node::CallbackScope>(env->GetIsolate(), resource, async);
+		auto& env = IsolateEnvironment::GetCurrent();
+		if (env.IsDefault()) {
+			scope = std::make_unique<node::CallbackScope>(env.GetIsolate(), resource, async);
 		}
 	}
 };
@@ -177,10 +176,10 @@ void ThreePhaseTask::Phase2Runner::Run() {
 		auto* holder = info.remotes.GetIsolateHolder();
 		holder->ScheduleTask(std::make_unique<Phase3Failure>(std::move(self), std::move(info), std::move(error)), false, true);
 	};
-	FunctorRunners::RunCatchExternal(IsolateEnvironment::GetCurrent()->DefaultContext(), [&]() {
+	FunctorRunners::RunCatchExternal(IsolateEnvironment::GetCurrent().DefaultContext(), [&]() {
 		// Continue the task
 		self->Phase2();
-		auto epilogue_error = IsolateEnvironment::GetCurrent()->TaskEpilogue();
+		auto epilogue_error = IsolateEnvironment::GetCurrent().TaskEpilogue();
 		if (epilogue_error) {
 			schedule_error(std::move(epilogue_error));
 		} else {
@@ -199,7 +198,7 @@ void ThreePhaseTask::Phase2RunnerIgnored::Run() {
 	TryCatch try_catch{Isolate::GetCurrent()};
 	try {
 		self->Phase2();
-		IsolateEnvironment::GetCurrent()->TaskEpilogue();
+		IsolateEnvironment::GetCurrent().TaskEpilogue();
 	} catch (const RuntimeError& cc_error) {}
 }
 
@@ -266,7 +265,7 @@ auto ThreePhaseTask::RunSync(IsolateHolder& second_isolate, bool allow_async) ->
 			}
 
 			// Run handle tasks for default isolate now
-			IsolateEnvironment& current_env = *IsolateEnvironment::GetCurrent();
+			auto& current_env = IsolateEnvironment::GetCurrent();
 			if (current_env.IsDefault()) {
 				run_handle_tasks(current_env);
 			}
@@ -317,7 +316,7 @@ auto ThreePhaseTask::RunSync(IsolateHolder& second_isolate, bool allow_async) ->
 
 				void Run() final {
 					did_run = true;
-					FunctorRunners::RunCatchExternal(IsolateEnvironment::GetCurrent()->DefaultContext(), [ this ]() {
+					FunctorRunners::RunCatchExternal(IsolateEnvironment::GetCurrent().DefaultContext(), [ this ]() {
 						// Now in the default thread
 						const auto is_async = [&]() {
 							if (allow_async) {
@@ -330,7 +329,7 @@ auto ThreePhaseTask::RunSync(IsolateHolder& second_isolate, bool allow_async) ->
 						if (!is_async) {
 							wait.Done();
 						}
-						this->error = IsolateEnvironment::GetCurrent()->TaskEpilogue();
+						this->error = IsolateEnvironment::GetCurrent().TaskEpilogue();
 					}, [ this ](unique_ptr<ExternalCopy> error) {
 						this->error = std::move(error);
 						wait.Done();
@@ -342,7 +341,7 @@ auto ThreePhaseTask::RunSync(IsolateHolder& second_isolate, bool allow_async) ->
 			unique_ptr<ExternalCopy> error;
 			{
 				// Setup condition variable to sleep this thread
-				IsolateEnvironment& env = *IsolateEnvironment::GetCurrent();
+				IsolateEnvironment& env = IsolateEnvironment::GetCurrent();
 				Scheduler::AsyncWait wait(*env.scheduler);
 				lockable_t<bool, false, true> done{false};
 				// Scope to unlock v8 in this thread and set up the wait

src/module/callback.cc

@@ -62,7 +62,7 @@ class InvokeRunner : public ThreePhaseTask {
 		void Phase2() final {
 			// Setup context
 			auto* isolate = Isolate::GetCurrent();
-			auto& env = *IsolateEnvironment::GetCurrent();
+			auto& env = IsolateEnvironment::GetCurrent();
 			auto context = Deref(data.context);
 			Context::Scope context_scope{context};
 			auto fn = Deref(data.callback);

src/module/context_handle.cc

@@ -117,10 +117,10 @@ class EvalRunner : public CodeCompilerHolder, public ThreePhaseTask {
 
 		void Phase2() final {
 			// Load script in and compile
-			auto* isolate = IsolateEnvironment::GetCurrent();
+			auto& isolate = IsolateEnvironment::GetCurrent();
 			auto context = this->context.Deref();
 			Context::Scope context_scope{context};
-			IsolateEnvironment::HeapCheck heap_check{*isolate, true};
+			IsolateEnvironment::HeapCheck heap_check{isolate, true};
 			auto source = GetSource();
 			auto script = RunWithAnnotatedErrors([&]() {
 				return Unmaybe(ScriptCompiler::Compile(context, source.get()));
@@ -185,10 +185,10 @@ class EvalClosureRunner : public CodeCompilerHolder, public ThreePhaseTask {
 
 		void Phase2() final {
 			// Setup isolate's context
-			auto* isolate = IsolateEnvironment::GetCurrent();
+			auto& isolate = IsolateEnvironment::GetCurrent();
 			auto context = this->context.Deref();
 			Context::Scope context_scope{context};
-			IsolateEnvironment::HeapCheck heap_check{*isolate, true};
+			IsolateEnvironment::HeapCheck heap_check{isolate, true};
 
 			// Generate $0 ... $N argument names
 			std::vector<Local<String>> argument_names;