Cleanup v8 on shutdown and add missed value emptiness check

Author: pinepain

php_v8.h

@@ -17,9 +17,11 @@
 #include "config.h"
 #endif
 
+#include <v8-version.h>
+#include <v8.h>
+
 extern "C" {
 #include "php.h"
-#include <v8-version.h>
 
 #ifdef ZTS
 #include "TSRM.h"
@@ -51,6 +53,7 @@ extern zend_module_entry php_v8_module_entry;
 
 ZEND_BEGIN_MODULE_GLOBALS(v8)
     bool v8_initialized;
+    v8::Platform *platform;
 ZEND_END_MODULE_GLOBALS(v8)
 
 // Add zend_type support (new since PHP 7.2)

src/php_v8_a.cc

@@ -29,28 +29,29 @@ void php_v8_init()
 
     v8::V8::InitializeICUDefaultLocation(PHP_V8_ICU_DATA_DIR);
 
-    // NOTE: if we use snapshot and extenal startup data then we have to initialize it (see https://codereview.chromium.org/315033002/)
+    // If we use snapshot and extenal startup data then we have to initialize it (see https://codereview.chromium.org/315033002/)
     // v8::V8::InitializeExternalStartupData(NULL);
     v8::Platform *platform = v8::platform::CreateDefaultPlatform();
     v8::V8::InitializePlatform(platform);
 
 //    const char *flags = "--no-hard_abort";
 //    v8::V8::SetFlagsFromString(flags, strlen(flags));
 
-
-    // TODO: remove flags?
-    /* Set V8 command line flags (must be done before V8::Initialize()!) */
-//    if (PHP_V8_G(v8_flags)) {
-//        v8::V8::SetFlagsFromString(PHP_V8_G(v8_flags), strlen(PHP_V8_G(v8_flags)));
-//    }
-
     /* Initialize V8 */
     v8::V8::Initialize();
 
     /* Run only once */
     PHP_V8_G(v8_initialized) = true;
+    PHP_V8_G(platform) = platform;
+}
+
+void php_v8_shutdown() {
+    if (!PHP_V8_G(v8_initialized)) {
+        return;
+    }
+
+    v8::V8::Dispose();
+    v8::V8::ShutdownPlatform();
 
-//    TODO: probably, not necessary to call it on shutdown
-//    v8::V8::Dispose();
-//    v8::V8::ShutdownPlatform();
+    delete PHP_V8_G(platform);
 }

src/php_v8_a.h

@@ -13,6 +13,15 @@
 #ifndef PHP_V8_A_H
 #define PHP_V8_A_H
 
+extern "C" {
+#include "php.h"
+
+#ifdef ZTS
+#include "TSRM.h"
+#endif
+};
+
 void php_v8_init();
+void php_v8_shutdown();
 
 #endif //PHP_V8_A_H

src/php_v8_isolate.cc

@@ -46,16 +46,13 @@ static void php_v8_maybe_terminate_execution(php_v8_isolate_t *php_v8_isolate) {
 static inline void php_v8_isolate_destroy(php_v8_isolate_t *php_v8_isolate) {
     if (php_v8_isolate->isolate) {
 
-        // TODO: terminate all executions!
         php_v8_maybe_terminate_execution(php_v8_isolate);
 
         while (php_v8_isolate->isolate->InContext()) {
             v8::Local<v8::Context> context = php_v8_isolate->isolate->GetEnteredContext();
-//            context->GetIsolate()->Exit();
             context->Exit();
         }
 
-        // TODO: exit if entered, TODO: maybe, move it to love above?
         if (php_v8_isolate->isolate == v8::Isolate::GetCurrent()) {
             php_v8_isolate->isolate->Exit();
         }
@@ -144,7 +141,6 @@ static zend_object *php_v8_isolate_ctor(zend_class_entry *ce) {
     zend_object_std_init(&php_v8_isolate->std, ce);
     object_properties_init(&php_v8_isolate->std, ce);
 
-    // TODO: inline? module init?
     php_v8_init();
 
     php_v8_isolate->create_params = new v8::Isolate::CreateParams();
@@ -422,7 +418,7 @@ static PHP_METHOD(V8Isolate, LowMemoryNotification) {
     PHP_V8_ISOLATE_FETCH_WITH_CHECK(getThis(), php_v8_isolate);
     PHP_V8_ENTER_ISOLATE(php_v8_isolate);
 
-    isolate->LowMemoryNotification(); // TODO: for some reason it reports memleak
+    isolate->LowMemoryNotification();
 }
 
 
@@ -460,7 +456,7 @@ static PHP_METHOD(V8Isolate, IsExecutionTerminating) {
     }
 
     PHP_V8_ISOLATE_FETCH_WITH_CHECK(getThis(), php_v8_isolate);
-    PHP_V8_ENTER_ISOLATE(php_v8_isolate); // TODO: can we just fetch isolate object here and do not eneter it?
+    PHP_V8_ENTER_ISOLATE(php_v8_isolate);
 
     RETURN_BOOL(isolate->IsExecutionTerminating());
 }
@@ -471,7 +467,7 @@ static PHP_METHOD(V8Isolate, CancelTerminateExecution) {
     }
 
     PHP_V8_ISOLATE_FETCH_WITH_CHECK(getThis(), php_v8_isolate);
-    PHP_V8_ENTER_ISOLATE(php_v8_isolate); // TODO: can we just fetch isolate object here and do not eneter it?
+    PHP_V8_ENTER_ISOLATE(php_v8_isolate);
 
     isolate->CancelTerminateExecution();
 }
@@ -488,7 +484,7 @@ static PHP_METHOD(V8Isolate, SetCaptureStackTraceForUncaughtExceptions) {
     PHP_V8_CHECK_STACK_TRACE_RANGE(frame_limit, "Frame limit is out of range");
 
     PHP_V8_ISOLATE_FETCH_WITH_CHECK(getThis(), php_v8_isolate);
-    PHP_V8_ENTER_ISOLATE(php_v8_isolate); // TODO: can we just fetch isolate object here and do not eneter it?
+    PHP_V8_ENTER_ISOLATE(php_v8_isolate);
 
     isolate->SetCaptureStackTraceForUncaughtExceptions(static_cast<bool>(capture),
                                                        static_cast<int>(frame_limit),

src/php_v8_script_compiler.cc

@@ -83,7 +83,7 @@ static PHP_METHOD(V8ScriptCompiler, CompileUnboundScript)
 
     PHP_V8_CHECK_COMPILER_OPTIONS_RANGE(options, "Invalid Script compiler options given. See V8\\ScriptCompiler\\CompileOptions class constants for available options.")
 
-    PHP_V8_CONTEXT_FETCH_INTO(php_v8_context_zv, php_v8_context);
+    PHP_V8_CONTEXT_FETCH_WITH_CHECK(php_v8_context_zv, php_v8_context);
 
     zval *source_string_zv = PHP_V8_SOURCE_READ_SOURCE_STRING(php_v8_source_zv);
     zval *origin_zv        = PHP_V8_SOURCE_READ_ORIGIN(php_v8_source_zv);
@@ -139,7 +139,7 @@ static PHP_METHOD(V8ScriptCompiler, Compile)
 
     PHP_V8_CHECK_COMPILER_OPTIONS_RANGE(options, "Invalid Script compiler options given. See V8\\ScriptCompiler\\CompileOptions class constants for available options.")
 
-    PHP_V8_CONTEXT_FETCH_INTO(php_v8_context_zv, php_v8_context);
+    PHP_V8_CONTEXT_FETCH_WITH_CHECK(php_v8_context_zv, php_v8_context);
 
     zval *source_string_zv = PHP_V8_SOURCE_READ_SOURCE_STRING(php_v8_source_zv);
     zval *origin_zv        = PHP_V8_SOURCE_READ_ORIGIN(php_v8_source_zv);
@@ -199,7 +199,7 @@ static PHP_METHOD(V8ScriptCompiler, CompileFunctionInContext)
         return;
     }
 
-    PHP_V8_CONTEXT_FETCH_INTO(php_v8_context_zv, php_v8_context);
+    PHP_V8_CONTEXT_FETCH_WITH_CHECK(php_v8_context_zv, php_v8_context);
 
     zval *source_string_zv = PHP_V8_SOURCE_READ_SOURCE_STRING(php_v8_source_zv);
     zval *origin_zv        = PHP_V8_SOURCE_READ_ORIGIN(php_v8_source_zv);

v8.cc

@@ -15,6 +15,7 @@
 #endif
 
 #include "php_v8.h"
+#include "php_v8_a.h"
 
 #include "php_v8_isolate.h"
 #include "php_v8_startup_data.h"
@@ -187,6 +188,7 @@ PHP_MSHUTDOWN_FUNCTION(v8)
     /* uncomment this line if you have INI entries
     UNREGISTER_INI_ENTRIES();
     */
+    php_v8_shutdown();
     return SUCCESS;
 }
 /* }}} */
@@ -240,6 +242,7 @@ static PHP_GINIT_FUNCTION(v8)
     ZEND_TSRMLS_CACHE_UPDATE();
 #endif
     v8_globals->v8_initialized = false;
+    v8_globals->platform = nullptr;
 }
 /* }}} */