fix: release persistent handle

Signed-off-by: Hosung Kim [email protected]

Author: hs0225

deps/node/src/lwnode/lwnode-public.cc

@@ -142,6 +142,11 @@ Runtime::~Runtime() {
 int Runtime::Start(int argc, char** argv, std::promise<void>&& promise) {
   LWNODE_PERF_LOG("[Runtime::Start]");
   LWNODE_DEV_LOG("[Runtime] version:", LWNODE_VERSION_TAG);
+#if defined(NDEBUG)
+  LWNODE_DEV_LOG("[Runtime] release mode");
+#else
+  LWNODE_DEV_LOG("[Runtime] debug mode");
+#endif
 
   internal_->runner_.SetInitPromise(std::move(promise));
   std::pair<bool, int> init_result = internal_->Init(argc, argv);

deps/node/src/node.cc

@@ -1109,11 +1109,13 @@ std::pair<bool, int> InitializeNode(int argc, char** argv, NodeMainInstance** ma
 }
 
 void DisposeNode(NodeMainInstance* main_instance) {
-  delete main_instance;
-
   LWNode::MessageLoop::GetInstance()->dispose();
 
+  main_instance->Dispose();
+
   TearDownOncePerProcess();
+
+  delete main_instance;
 }
 #endif
 

deps/node/src/node_main_instance.cc

@@ -1,7 +1,7 @@
 #include <memory>
 
-#include "node_main_instance.h"
 #include "node_internals.h"
+#include "node_main_instance.h"
 #include "node_options-inl.h"
 #include "node_v8_platform-inl.h"
 #include "util-inl.h"
@@ -92,18 +92,18 @@ NodeMainInstance::NodeMainInstance(
 }
 
 void NodeMainInstance::Dispose() {
-  CHECK(!owns_isolate_);
-  platform_->DrainTasks(isolate_);
-}
-
-NodeMainInstance::~NodeMainInstance() {
   if (!owns_isolate_) {
     return;
   }
+
+  platform_->DrainTasks(isolate_);
+
   platform_->UnregisterIsolate(isolate_);
   isolate_->Dispose();
 }
 
+NodeMainInstance::~NodeMainInstance() {}
+
 int NodeMainInstance::Run() {
   Locker locker(isolate_);
   Isolate::Scope isolate_scope(isolate_);
@@ -158,8 +158,7 @@ int NodeMainInstance::Run() {
   struct sigaction act;
   memset(&act, 0, sizeof(act));
   for (unsigned nr = 1; nr < kMaxSignal; nr += 1) {
-    if (nr == SIGKILL || nr == SIGSTOP || nr == SIGPROF)
-      continue;
+    if (nr == SIGKILL || nr == SIGSTOP || nr == SIGPROF) continue;
     act.sa_handler = (nr == SIGPIPE) ? SIG_IGN : SIG_DFL;
     CHECK_EQ(0, sigaction(nr, &act, nullptr));
   }
@@ -197,12 +196,12 @@ NodeMainInstance::CreateMainEnvironment(int* exit_code) {
   CHECK(!context.IsEmpty());
   Context::Scope context_scope(context);
 
-  DeleteFnPtr<Environment, FreeEnvironment> env { CreateEnvironment(
-      isolate_data_.get(),
-      context,
-      args_,
-      exec_args_,
-      EnvironmentFlags::kDefaultFlags) };
+  DeleteFnPtr<Environment, FreeEnvironment> env{
+      CreateEnvironment(isolate_data_.get(),
+                        context,
+                        args_,
+                        exec_args_,
+                        EnvironmentFlags::kDefaultFlags)};
 
   if (*exit_code != 0) {
     return env;

deps/node/src/node_main_lw.cc

@@ -72,7 +72,6 @@ int main(int argc, char* argv[]) {
   setvbuf(stderr, nullptr, _IONBF, 0);
 
   lwnode::Runtime runtime;
-  std::pair<bool, int> init_result;  // early_return, exit_code
 
   // FIXME: Fix Runtime::Init() call to ensure environment initialization
   // before running the loop, Runtime::Run(). This workaround passes a

include/lwnode/lwnode-version.h

@@ -18,5 +18,5 @@
 
  #define LWNODE_VERSION_MAJOR 1
  #define LWNODE_VERSION_MINOR 0
- #define LWNODE_VERSION_PATCH 1
- #define LWNODE_VERSION_TAG "v1.0.1"
+ #define LWNODE_VERSION_PATCH 2
+ #define LWNODE_VERSION_TAG "v1.0.2"

packaging/lwnode.spec

@@ -126,8 +126,8 @@ echo $CFLAGS
 
 # building lwnode executable
 
-CFLAGS+=" -fno-lto -DMAX_HEAP_SECTS=7680 "
-CXXFLAGS+=" -fno-lto -DMAX_HEAP_SECTS=7680 "
+CFLAGS+=" -g -fno-lto -DMAX_HEAP_SECTS=7680 "
+CXXFLAGS+=" -g -fno-lto -DMAX_HEAP_SECTS=7680 "
 LDFLAGS+=" -fno-lto "
 
 ./tools/envinfo.sh
@@ -141,7 +141,7 @@ LDFLAGS+=" -fno-lto "
             --arch='%{tizen_arch}' \
             %{?lib_type_config} %{?asan_config} \
             %{?external_libs_config} %{?jsengine_config} \
-            --debug --escargot-debugger
+            --debug
 ninja -C %{target_src} %{target}
 %undefine target_src
 %endif

src/api-scripts.cc

@@ -291,7 +291,8 @@ MaybeLocal<UnboundScript> ScriptCompiler::CompileUnboundInternal(
     esResourceName = VAL(*source->resource_name)->value()->asString();
   }
 
-  ContextRef* esPureContext = ContextRef::create(lwIsolate->vmInstance());
+  PersistentRefHolder<Escargot::ContextRef> esPureContext =
+      ContextRef::create(lwIsolate->vmInstance());
   ScriptParserRef* parser = esPureContext->scriptParser();
   ScriptParserRef::InitializeScriptResult result = parser->initializeScript(
       esSource, esResourceName, source->GetResourceOptions().IsModule());
@@ -303,6 +304,8 @@ MaybeLocal<UnboundScript> ScriptCompiler::CompileUnboundInternal(
 
     lwIsolate->SetPendingExceptionAndMessage(r.error.get(), r.stackTrace);
     lwIsolate->ReportPendingMessages();
+
+    esPureContext.release();
     return MaybeLocal<UnboundScript>();
   }
 
@@ -418,12 +421,15 @@ MaybeLocal<Function> ScriptCompiler::CompileFunctionInContext(
   // note: expand API_HANDLE_EXCEPTION and add the resource name
   if (!result.isSuccessful()) {
     Evaluator::EvaluatorResult r;
-    ContextRef* esPureContext = ContextRef::create(lwIsolate->vmInstance());
+    PersistentRefHolder<Escargot::ContextRef> esPureContext =
+        ContextRef::create(lwIsolate->vmInstance());
     r.error = ExceptionHelper::createErrorObject(
         esPureContext, result.parseErrorCode, result.parseErrorMessage);
 
     lwIsolate->SetPendingExceptionAndMessage(r.error.get(), r.stackTrace);
     lwIsolate->ReportPendingMessages();
+
+    esPureContext.release();
     return MaybeLocal<Function>();
   }
 

src/api/context.cc

@@ -68,6 +68,8 @@ ContextWrap::ContextWrap(IsolateWrap* isolate,
   EscargotShim::Global::initGlobalObject(this);
 
   RegisteredExtension::applyAll(context_);
+
+  isolate->AddContextClenupHook(this);
 }
 
 void ContextWrap::initDebugger() {
@@ -109,6 +111,10 @@ void ContextWrap::Exit() {
   isolate_->popContext(this);
 }
 
+void ContextWrap::Dispose() {
+  context_.release();
+}
+
 IsolateWrap* ContextWrap::GetIsolate() {
   return isolate_;
 }

src/api/context.h

@@ -38,6 +38,8 @@ class ContextWrap : public ValueWrap {
   void Exit();
   IsolateWrap* GetIsolate();
 
+  void Dispose();
+
   Escargot::ContextRef* get() { return context_; }
 
   void SetEmbedderData(int index, ValueWrap* value);
@@ -70,7 +72,7 @@ class ContextWrap : public ValueWrap {
   void* getEmbedderData(int index);
 
   IsolateWrap* isolate_ = nullptr;
-  Escargot::ContextRef* context_ = nullptr;
+  Escargot::PersistentRefHolder<Escargot::ContextRef> context_;
   Escargot::ObjectRef* bindingObject_ = nullptr;
   Escargot::ValueRef* security_token_ = nullptr;
 

src/api/engine.cc

@@ -409,8 +409,10 @@ void Engine::dispose() {
   gcHeap_.release();
   GC_invoke_finalizers();
 
-  Globals::finalize();
   disposeExternalStrings();
+
+  MemoryUtil::gcFull();
+  Globals::finalize();
   LWNODE_CALL_TRACE_GC_END();
 }
 

src/api/isolate.cc

@@ -278,7 +278,7 @@ IsolateWrap::IsolateWrap() {
       SymbolRef::create(StringRef::createFromUTF8(PRIVATE_VALUES.data(),
                                                   PRIVATE_VALUES.length())));
 
-  threadManager_ = new ThreadManager();
+  thread_manager_ = new ThreadManager();
 
   // NOTE: check lock_gc_release(); is needed (and where)
   // lock_gc_release();
@@ -307,6 +307,10 @@ IsolateWrap::IsolateWrap() {
 
 IsolateWrap::~IsolateWrap() {
   LWNODE_CALL_TRACE_ID(ISOWRAP, "free: %p", this);
+  vmInstance_.release();
+
+  s_currentIsolate = nullptr;
+  s_previousIsolate = nullptr;
 }
 
 IsolateWrap* IsolateWrap::New() {
@@ -320,7 +324,15 @@ void IsolateWrap::Dispose() {
   // NOTE: check unlock_gc_release(); is needed (and where)
   // unlock_gc_release();
 
+
+  privateValuesSymbol_.release();
   global_handles()->dispose();
+
+  delete thread_manager_;
+  thread_manager_ = nullptr;  
+
+  ReleaseContexts();
+
   RegisteredExtension::unregisterAll();
 
   state_ = State::Disposed;
@@ -857,4 +869,15 @@ void IsolateWrap::ReportPromiseReject(
   }
 }
 
+void IsolateWrap::AddContextClenupHook(ContextWrap* context) {
+  contexts_.push_back(context);
+}
+
+void IsolateWrap::ReleaseContexts() {
+  for (const auto& context : contexts_) {
+    context->Dispose();
+  }
+  contexts_.clear();
+}
+
 }  // namespace EscargotShim

src/api/isolate.h

@@ -271,19 +271,23 @@ class IsolateWrap final : public v8::internal::Isolate {
                            Escargot::ValueRef* value,
                            Escargot::VMInstanceRef::PromiseRejectEvent event);
 
-  ThreadManager* thread_manager() { return threadManager_; }
+  ThreadManager* thread_manager() { return thread_manager_; }
 
   void PerformMicrotaskCheckpoint() {
     v8::MicrotasksScope::PerformCheckpoint(toV8(this));
   }
 
   State getState() { return state_; }
 
+  void AddContextClenupHook(ContextWrap* context);
+  void ReleaseContexts();
+
  private:
   IsolateWrap();
 
   void InitializeGlobalSlots();
 
+  GCVector<ContextWrap*> contexts_;
   GCVector<GCManagedObject*> eternals_;
   GCMap<BackingStoreRef*, int, BackingStoreComparator> backingStoreCounter_;
 
@@ -302,12 +306,12 @@ class IsolateWrap final : public v8::internal::Isolate {
   v8::ArrayBuffer::Allocator* array_buffer_allocator_ = nullptr;
   std::shared_ptr<v8::ArrayBuffer::Allocator> array_buffer_allocator_shared_;
 
-  VMInstanceRef* vmInstance_ = nullptr;
+  PersistentRefHolder<VMInstanceRef> vmInstance_;
 
   PersistentRefHolder<IsolateWrap> release_lock_;
   ValueWrap* globalSlot_[internal::Internals::kRootIndexSize]{};
 
-  ThreadManager* threadManager_ = nullptr;
+  ThreadManager* thread_manager_ = nullptr;
 
   v8::PromiseRejectCallback promise_reject_callback_{nullptr};
 

src/execution/v8threads.h

@@ -24,7 +24,7 @@
 
 namespace EscargotShim {
 
-class ThreadManager : public gc {
+class ThreadManager {
  public:
   void Lock();
   void Unlock();