implement per-fork exceptions

VKCOM · Feb 19, 2025 · de0de55 · de0de55
1 parent ba62015
commit de0de55
Show file tree

Hide file tree

Showing 9 changed files with 41 additions and 52 deletions.
diff --git a/compiler/code-gen/vertex-compiler.cpp b/compiler/code-gen/vertex-compiler.cpp
@@ -309,7 +309,7 @@ void compile_throw(VertexAdaptor<op_throw> root, CodeGenerator &W) {
 }
 
 void compile_try(VertexAdaptor<op_try> root, CodeGenerator &W) {
-  std::string_view cur_exception{G->is_output_mode_k2() ? "ExceptionInstanceState::get().cur_exception" : "CurException"};
+  std::string_view cur_exception{G->is_output_mode_k2() ? "ForkInstanceState::get().current_info().get().thrown_exception" : "CurException"};
 
   auto move_exception = [&](ClassPtr caught_class, VertexAdaptor<op_var> dst) {
     if (caught_class->name == "Throwable") {

diff --git a/runtime-light/coroutine/awaitable.h b/runtime-light/coroutine/awaitable.h
@@ -286,7 +286,7 @@ class start_fork_t : awaitable_impl_::fork_id_watcher_t {
   explicit start_fork_t(task_t<T> &&task) noexcept
     : fork_id(fork_instance_st.push_fork(
         static_cast<shared_task_t<void>>(std::invoke([](task_t<T> task) noexcept -> shared_task_t<T> { co_return co_await task; }, std::move(task)))))
-    , fork_awaiter(std::get<shared_task_t<void>>((*fork_instance_st.get_info(fork_id)).handle).when_ready()) {}
+    , fork_awaiter(std::get<shared_task_t<void>>((*fork_instance_st.get_info(fork_id)).get().handle).when_ready()) {}
 
   start_fork_t(start_fork_t &&other) noexcept
     : fork_instance_st(other.fork_instance_st)

diff --git a/runtime-light/state/instance-state.cpp b/runtime-light/state/instance-state.cpp
@@ -5,6 +5,7 @@
 #include "runtime-light/state/instance-state.h"
 
 #include <chrono>
+#include <cinttypes>
 #include <cstdint>
 #include <functional>
 #include <memory>
@@ -51,8 +52,16 @@ void InstanceState::init_script_execution() noexcept {
 
   auto main_task{std::invoke(
     [](task_t<void> script_task) noexcept -> task_t<void> {
-      auto fork_id{co_await start_fork_t{std::move(script_task)}};
-      php_assert(co_await f$wait_concurrently(fork_id)); // i'd better use f$wait here, but we need to enable its void instantiation first
+      // wrap script with additional check for unhandled exception
+      script_task = std::invoke(
+        [](task_t<void> script_task) noexcept -> task_t<void> {
+          co_await script_task;
+          if (auto exception{ForkInstanceState::get().current_info().get().thrown_exception}; !exception.is_null()) [[unlikely]] {
+            php_error("unhandled exception '%s' at %s:%" PRId64, exception.get_class(), exception->$file.c_str(), exception->$line);
+          }
+        },
+        std::move(script_task));
+      php_assert(co_await f$wait_concurrently(co_await start_fork_t{std::move(script_task)}));
     },
     std::move(script_task))};
   scheduler.suspend(std::make_pair(main_task.get_handle(), WaitEvent::Rechedule{}));

diff --git a/runtime-light/state/instance-state.h b/runtime-light/state/instance-state.h
@@ -22,7 +22,6 @@
 #include "runtime-light/server/job-worker/job-worker-server-state.h"
 #include "runtime-light/stdlib/crypto/crypto-state.h"
 #include "runtime-light/stdlib/curl/curl-state.h"
-#include "runtime-light/stdlib/diagnostics/exception-state.h"
 #include "runtime-light/stdlib/file/file-system-state.h"
 #include "runtime-light/stdlib/fork/fork-state.h"
 #include "runtime-light/stdlib/job-worker/job-worker-client-state.h"
@@ -111,7 +110,6 @@ struct InstanceState final : vk::not_copyable {
 
   RuntimeContext runtime_context;
   RpcInstanceState rpc_instance_state;
-  ExceptionInstanceState exception_instance_state;
   SerializationInstanceState serialization_instance_state;
   HttpServerInstanceState http_server_instance_state;
   JobWorkerClientInstanceState job_worker_client_instance_state{};

diff --git a/runtime-light/stdlib/diagnostics/exception-functions.h b/runtime-light/stdlib/diagnostics/exception-functions.h
@@ -13,13 +13,11 @@
 #define THROW_EXCEPTION(e)                                                                                                                                     \
   {                                                                                                                                                            \
     Throwable x_tmp___ = e;                                                                                                                                    \
-    auto &cur_exception{ExceptionInstanceState::get().cur_exception};                                                                                          \
-    php_assert(cur_exception.is_null());                                                                                                                       \
-    cur_exception = std::move(x_tmp___);                                                                                                                       \
+    php_assert(std::exchange(ForkInstanceState::get().current_info().get().thrown_exception, std::move(x_tmp___)).is_null());                                  \
   }
 
 #define CHECK_EXCEPTION(action)                                                                                                                                \
-  if (!ExceptionInstanceState::get().cur_exception.is_null()) [[unlikely]] {                                                                                   \
+  if (!ForkInstanceState::get().current_info().get().thrown_exception.is_null()) [[unlikely]] {                                                                \
     action;                                                                                                                                                    \
   }
 

diff --git a/runtime-light/stdlib/diagnostics/exception-state.cpp b/runtime-light/stdlib/diagnostics/exception-state.cpp
diff --git a/runtime-light/stdlib/diagnostics/exception-state.h b/runtime-light/stdlib/diagnostics/exception-state.h
diff --git a/runtime-light/stdlib/fork/fork-functions.h b/runtime-light/stdlib/fork/fork-functions.h
@@ -5,8 +5,10 @@
 #pragma once
 
 #include <chrono>
+#include <cinttypes>
 #include <concepts>
 #include <cstdint>
+#include <optional>
 #include <utility>
 #include <variant>
 
@@ -38,15 +40,23 @@ template<typename T>
 requires(is_optional<T>::value || std::same_as<T, mixed> || is_class_instance<T>::value) task_t<T> f$wait(int64_t fork_id, double timeout = -1.0) noexcept {
   auto &fork_instance_st{ForkInstanceState::get()};
   auto opt_fork_info{fork_instance_st.get_info(fork_id)};
-  if (!opt_fork_info.has_value() || std::holds_alternative<std::monostate>((*opt_fork_info).handle)) [[unlikely]] {
+  if (!opt_fork_info.has_value() || std::holds_alternative<std::monostate>((*opt_fork_info).get().handle)) [[unlikely]] {
     php_warning("fork with ID %" PRId64 " does not exist or has already been awaited by another fork", fork_id);
     co_return T{};
   }
 
-  auto fork_task{static_cast<shared_task_t<internal_optional_type_t<T>>>(std::move(std::get<shared_task_t<void>>((*opt_fork_info).handle)))};
-  auto opt_result{co_await wait_with_timeout_t{wait_fork_t{std::move(fork_task)}, forks_impl_::normalize_timeout(timeout)}};
-  // remove shared_task_t from ForkInstanceState
-  fork_instance_st.clear_fork(fork_id);
+  auto &fork_info{(*opt_fork_info).get()};
+  auto fork_task{std::get<shared_task_t<void>>(fork_info.handle)};
+  auto opt_result{co_await wait_with_timeout_t{wait_fork_t{static_cast<shared_task_t<internal_optional_type_t<T>>>(std::move(fork_task))},
+                                               forks_impl_::normalize_timeout(timeout)}};
+  // Execute essential housekeeping tasks to maintain proper state management.
+  // 1. Check for any exceptions that may have occurred during the fork execution. If an exception is found, propagate it to the current fork.
+  //    Clean fork_info's exception state.
+  auto current_fork_info{fork_instance_st.current_info()};
+  php_assert(std::exchange(current_fork_info.get().thrown_exception, std::move(fork_info.thrown_exception)).is_null());
+  // 2. Detach the shared_task_t from fork_info to prevent further associations, ensuring that resources are released.
+  fork_info.handle.emplace<std::monostate>();
+
   co_return opt_result.has_value() ? T{std::move(opt_result.value())} : T{};
 }
 
@@ -65,9 +75,9 @@ inline task_t<bool> f$wait_concurrently(int64_t fork_id) noexcept {
     co_return false;
   }
 
-  auto fork_info{*std::move(opt_fork_info)};
+  const auto &fork_info{(*opt_fork_info).get()};
   if (std::holds_alternative<shared_task_t<void>>(fork_info.handle)) {
-    auto fork_task{std::move(std::get<shared_task_t<void>>(fork_info.handle))};
+    auto fork_task{std::get<shared_task_t<void>>(fork_info.handle)};
     co_await wait_fork_t{std::move(fork_task)};
   }
   co_return true;

diff --git a/runtime-light/stdlib/fork/fork-state.h b/runtime-light/stdlib/fork/fork-state.h
@@ -5,13 +5,15 @@
 #pragma once
 
 #include <cstdint>
+#include <functional>
 #include <optional>
 #include <utility>
 #include <variant>
 
 #include "common/mixin/not_copyable.h"
 #include "runtime-common/core/allocator/script-allocator.h"
 #include "runtime-common/core/std/containers.h"
+#include "runtime-common/core/utils/kphp-assert-core.h"
 #include "runtime-light/coroutine/shared-task.h"
 #include "runtime-light/stdlib/diagnostics/exception-types.h"
 
@@ -26,7 +28,7 @@ struct ForkInstanceState final : private vk::not_copyable {
   // In the future, we plan to implement a reference-counted future<T> that will automatically
   // manage and destroy the fork state once all referencing futures have been destroyed.
   struct fork_info final {
-    std::optional<Throwable> thrown_exception;
+    Throwable thrown_exception;
     std::variant<std::monostate, shared_task_t<void>> handle;
   };
 
@@ -38,7 +40,7 @@ struct ForkInstanceState final : private vk::not_copyable {
   kphp::stl::unordered_map<int64_t, fork_info, kphp::memory::script_allocator> forks;
 
   int64_t push_fork(shared_task_t<void> fork_task) noexcept {
-    forks.emplace(next_fork_id, fork_info{.thrown_exception = std::nullopt, .handle = std::move(fork_task)});
+    forks.emplace(next_fork_id, fork_info{.thrown_exception = {}, .handle = std::move(fork_task)});
     return next_fork_id++;
   }
 
@@ -52,18 +54,17 @@ struct ForkInstanceState final : private vk::not_copyable {
 
   static ForkInstanceState &get() noexcept;
 
-  std::optional<fork_info> get_info(int64_t fork_id) const noexcept {
+  std::optional<std::reference_wrapper<fork_info>> get_info(int64_t fork_id) noexcept {
     if (auto it{forks.find(fork_id)}; it != forks.end()) [[likely]] {
       return it->second;
     }
     return std::nullopt;
   }
 
-  void clear_fork(int64_t fork_id) noexcept {
-    auto it{forks.find(fork_id)};
-    if (it == forks.end()) [[unlikely]] {
-      return;
+  std::reference_wrapper<fork_info> current_info() noexcept {
+    if (auto opt_fork_info{get_info(current_id)}; opt_fork_info.has_value()) [[likely]] {
+      return *opt_fork_info;
     }
-    it->second.handle.emplace<std::monostate>();
+    php_critical_error("can't find info for current fork");
   }
 };