BUG#37694461 Commands out of sync after executing JavaScript using getSession

The problem was caused because the result was not properly cleaned up
when the execution of the script terminated.

Change-Id: Ica6afb418d32ec41bc02221d2115168109ef47a2

Author: rennox

router/src/jit_executor/src/utils/utils_general.h renamed to router/src/harness/include/mysql/harness/scoped_callback.h

@@ -23,46 +23,42 @@
  * 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
-#ifndef MYSQLSHDK_LIBS_UTILS_UTILS_GENERAL_H_
-#define MYSQLSHDK_LIBS_UTILS_UTILS_GENERAL_H_
+#ifndef ROUTER_SRC_HARNESS_INCLUDE_MYSQL_HARNESS_SCOPED_CALLBACK_H_
+#define ROUTER_SRC_HARNESS_INCLUDE_MYSQL_HARNESS_SCOPED_CALLBACK_H_
 
 #include <functional>
-#include <stdexcept>
-#include <string_view>
 #include <utility>
 
-namespace shcore {
+namespace mysql_harness {
 
-class Scoped_callback {
+class ScopedCallback {
  public:
-  explicit Scoped_callback(std::function<void()> c) noexcept
-      : m_callback{std::move(c)} {}
+  explicit ScopedCallback(std::function<void()> c) noexcept
+      : callback_{std::move(c)} {}
 
-  Scoped_callback() = default;
-  Scoped_callback(const Scoped_callback &) = delete;
-  Scoped_callback &operator=(const Scoped_callback &) = delete;
+  ScopedCallback() = default;
+  ScopedCallback(const ScopedCallback &) = delete;
+  ScopedCallback &operator=(const ScopedCallback &) = delete;
 
-  Scoped_callback(Scoped_callback &&o) noexcept { *this = std::move(o); }
-  Scoped_callback &operator=(Scoped_callback &&o) noexcept {
-    if (this != &o) std::swap(m_callback, o.m_callback);
+  ScopedCallback(ScopedCallback &&o) noexcept { *this = std::move(o); }
+  ScopedCallback &operator=(ScopedCallback &&o) noexcept {
+    if (this != &o) std::swap(callback_, o.callback_);
     return *this;
   }
 
-  ~Scoped_callback() noexcept;
+  ~ScopedCallback() noexcept;
 
   void call() {
-    if (!m_callback) return;
-    std::exchange(m_callback, nullptr)();
+    if (!callback_) return;
+    std::exchange(callback_, nullptr)();
   }
 
-  void cancel() { m_callback = nullptr; }
+  void cancel() { callback_ = nullptr; }
 
  private:
-  std::function<void()> m_callback;
+  std::function<void()> callback_;
 };
 
-bool is_valid_identifier(std::string_view name);
+}  // namespace mysql_harness
 
-}  // namespace shcore
-
-#endif  // MYSQLSHDK_LIBS_UTILS_UTILS_GENERAL_H_
+#endif  // ROUTER_SRC_HARNESS_INCLUDE_MYSQL_HARNESS_SCOPED_CALLBACK_H_

router/src/harness/src/CMakeLists.txt

@@ -262,6 +262,7 @@ SET(harness_source
   log_reopen.cc
   random_generator.cc
   signal_handler.cc
+  scoped_callback.cc
   sd_notify.cc
   socket_operations.cc
   tls_cipher.cc

router/src/jit_executor/src/utils/utils_general.cc renamed to router/src/harness/src/scoped_callback.cc

@@ -23,36 +23,22 @@
  * 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
-#include "utils/utils_general.h"
+#include "mysql/harness/scoped_callback.h"
 
-#include <regex>
-#include <string_view>
-#include <utility>
+#include <exception>
 
 #include "mysql/harness/logging/logging.h"
 
-namespace shcore {
+namespace mysql_harness {
 
-Scoped_callback::~Scoped_callback() noexcept {
+IMPORT_LOG_FUNCTIONS()
+
+ScopedCallback::~ScopedCallback() noexcept {
   try {
     call();
   } catch (const std::exception &e) {
-    mysql_harness::logging::log_error("Unexpected exception: %s", e.what());
+    log_error("Unexpected exception: %s", e.what());
   }
 }
 
-bool is_valid_identifier(std::string_view name) {
-  if (name.empty()) return false;
-
-  std::locale locale;
-
-  if (!std::isalpha(name.front(), locale) && (name.front() != '_'))
-    return false;
-
-  return std::all_of(
-      std::next(name.begin()), name.end(), [&locale](auto cur_char) {
-        return std::isalnum(cur_char, locale) || (cur_char == '_');
-      });
-}
-
-}  // namespace shcore
+}  // namespace mysql_harness

router/src/jit_executor/src/CMakeLists.txt

@@ -59,7 +59,6 @@ SET(jit_executor_plugin_SOURCE
   utils/polyglot_utils.cc
   utils/profiling.cc
   utils/utils_encoding.cc
-  utils/utils_general.cc
   utils/utils_json.cc
   utils/utils_path.cc
   utils/utils_string.cc

router/src/jit_executor/src/database/session.cc

@@ -35,10 +35,10 @@
 // #include "mysqlshdk/libs/utils/debug.h"
 // #include "mysqlshdk/libs/utils/fault_injection.h"
 // #include "mysqlshdk/libs/utils/log_sql.h"
+#include "mysql/harness/scoped_callback.h"
 #include "objects/polyglot_date.h"
 #include "router/src/router/include/mysqlrouter/mysql_session.h"
 #include "utils/profiling.h"
-#include "utils/utils_general.h"
 
 namespace shcore {
 namespace polyglot {
@@ -150,7 +150,7 @@ void Session::reset() {
 std::shared_ptr<IResult> Session::run_sql(const std::string &sql) {
   auto attributes = query_attributes();
 
-  shcore::Scoped_callback clean_query_attributes(
+  mysql_harness::ScopedCallback clean_query_attributes(
       [this]() { m_query_attributes.clear(); });
 
   return run_sql(sql.c_str(), sql.size(), false, false, attributes);

router/src/jit_executor/src/database/session.h

@@ -62,6 +62,9 @@ class JIT_EXECUTOR_PLUGIN_EXPORT Session
   Session(MYSQL *mysql);
   ~Session() override;
 
+  // Ensures the connection is ready for the next query execution
+  void reset() override;
+
  private:
   std::shared_ptr<IResult> query(
       const char *sql, size_t len, bool buffered,
@@ -134,9 +137,6 @@ class JIT_EXECUTOR_PLUGIN_EXPORT Session
   void set_query_attributes(const shcore::Dictionary_t &args);
   std::vector<Query_attribute> query_attributes() const;
 
-  // Ensures the connection is ready for the next query execution
-  void reset() override;
-
   std::shared_ptr<IResult> run_sql(const std::string &sql) override;
 
   std::shared_ptr<IResult> run_sql(

router/src/jit_executor/src/jit_executor_javascript.cc

@@ -38,14 +38,14 @@
 #include "include/my_thread.h"
 #include "languages/polyglot_javascript.h"
 #include "mysql/harness/logging/logging.h"
+#include "mysql/harness/scoped_callback.h"
 #include "mysqlrouter/jit_executor_common.h"
 #include "mysqlrouter/jit_executor_db_interface.h"
 #include "mysqlrouter/polyglot_file_system.h"
 #include "objects/polyglot_date.h"
 #include "objects/polyglot_session.h"
 #include "utils/polyglot_error.h"
 #include "utils/polyglot_utils.h"
-#include "utils/utils_general.h"
 #include "utils/utils_string.h"
 
 namespace jit_executor {
@@ -257,7 +257,7 @@ void JavaScript::run() {
     set_processing_state(ProcessingState::Finished);
   }
 
-  shcore::Scoped_callback terminate([this, &initialized]() {
+  mysql_harness::ScopedCallback terminate([this, &initialized]() {
     if (initialized) {
       try {
         finalize();
@@ -516,7 +516,7 @@ std::string JavaScript::execute(const std::string &code, int timeout,
   m_global_callbacks = &global_callbacks;
   m_code.push(Code{code, result_type});
 
-  shcore::Scoped_callback clean_resources([this]() {
+  mysql_harness::ScopedCallback clean_resources([this]() {
     if (m_session) {
       m_session->reset();
     }

router/src/jit_executor/src/languages/polyglot_common_context.cc

@@ -27,10 +27,10 @@
 
 #include <cstring>
 
+#include "mysql/harness/scoped_callback.h"
 #include "native_wrappers/polyglot_collectable.h"
 #include "utils/polyglot_scope.h"
 #include "utils/polyglot_utils.h"
-#include "utils/utils_general.h"
 
 namespace shcore {
 namespace polyglot {
@@ -60,7 +60,7 @@ void Polyglot_common_context::initialize(
     const std::vector<std::string> &isolate_args) {
   if (!isolate_args.empty()) {
     char **params = get_char_ptr(isolate_args);
-    shcore::Scoped_callback release([&]() { delete[] params; });
+    mysql_harness::ScopedCallback release([&]() { delete[] params; });
 
     poly_isolate_params isolate_params;
     if (poly_ok != poly_set_isolate_params(&isolate_params,

router/src/jit_executor/src/languages/polyglot_language.cc

@@ -37,10 +37,10 @@
 
 #include "languages/polyglot_common_context.h"
 #include "mysql/harness/logging/logging.h"
+#include "mysql/harness/scoped_callback.h"
 #include "native_wrappers/polyglot_file_system_wrapper.h"
 #include "utils/polyglot_error.h"
 #include "utils/polyglot_scope.h"
-#include "utils/utils_general.h"
 #include "utils/utils_path.h"
 #include "utils/utils_string.h"
 
@@ -204,7 +204,7 @@ void Polyglot_language::finalize() {
   m_globals.reset();
   m_types->dispose();
 
-  shcore::Scoped_callback cleanup{[this]() {
+  mysql_harness::ScopedCallback cleanup{[this]() {
     m_context.reset();
     m_storage->clear();
     m_scope->close();

router/src/jit_executor/src/objects/polyglot_row.cc

@@ -33,7 +33,6 @@
 #include "mysqlrouter/jit_executor_db_interface.h"
 #include "mysqlrouter/jit_executor_exceptions.h"
 #include "objects/polyglot_date.h"
-#include "utils/utils_general.h"
 
 namespace shcore {
 namespace polyglot {

router/src/mysql_rest_service/src/mrs/endpoint/handler/handler_db_object_script.cc

@@ -50,6 +50,7 @@
 #include "mrs/rest/request_context.h"
 #include "mrs/rest/response_cache.h"
 #include "mrs/router_observation_entities.h"
+#include "mysql/harness/scoped_callback.h"
 #include "router/src/mysql_rest_service/include/collector/mysql_cache_manager.h"
 
 #ifdef HAVE_JIT_EXECUTOR_PLUGIN
@@ -144,7 +145,7 @@ class HandlerDbObjectScript::Impl {
 
       if (params.get_type() != shcore::Map) {
         throw http::Error(HttpStatusCode::BadRequest,
-                          "Invalid parameters format");
+                          "Invalid parameters format.");
       }
 
       auto params_map = params.as_map();
@@ -166,12 +167,16 @@ class HandlerDbObjectScript::Impl {
           allowed_params.push_back(it.name);
         }
 
-        auto allowed_str = shcore::str_join(allowed_params, ", ");
+        std::string allowed_str{"The function accepts no parameters."};
+        if (!allowed_params.empty()) {
+          allowed_str =
+              "Allowed: " + shcore::str_join(allowed_params, ", ") + ".";
+        }
         auto invalid_str = shcore::str_join(invalid_params, ", ");
 
-        throw http::Error(HttpStatusCode::BadRequest,
-                          "Not allowed parameter:"s + invalid_str +
-                              ". Allowed: " + allowed_str);
+        throw http::Error(
+            HttpStatusCode::BadRequest,
+            "Not allowed parameter:"s + invalid_str + ". " + allowed_str);
       }
 
       for (auto &el : fields) {
@@ -285,6 +290,15 @@ HttpResult HandlerDbObjectScript::handle_script(
       }
 
       HandlerDbObjectTable::CachedSession session;
+      std::shared_ptr<shcore::polyglot::database::Session> be_session;
+
+      mysql_harness::ScopedCallback reset_session([&be_session]() {
+        // This reset ensures the session is left in a usable state
+        if (be_session) {
+          be_session->reset();
+        }
+      });
+
       auto result = context->get()->execute(
           m_impl->entry_script(), entry_->content_set_def->class_name,
           entry_->content_set_def->name, parameters, timeout, result_type,
@@ -301,8 +315,10 @@ HttpResult HandlerDbObjectScript::handle_script(
              session = get_session(ctxt, session_type);
              session->connection_id();
 
-             return std::make_shared<shcore::polyglot::database::Session>(
+             be_session = std::make_shared<shcore::polyglot::database::Session>(
                  session.get()->get_handle());
+
+             return be_session;
            },
            // Get current MRS User ID Callback
            [&, ctxt]() -> std::optional<std::string> {

router/tests/mrs_client/main.cc

@@ -1,5 +1,5 @@
 /*
-  Copyright (c) 2023, 2024, Oracle and/or its affiliates.
+  Copyright (c) 2023, 2025, Oracle and/or its affiliates.
 
   This program is free software; you can redistribute it and/or modify
   it under the terms of the GNU General Public License, version 2.0,
@@ -229,6 +229,7 @@ static const std::map<std::string, HttpStatusCode::key_type>
       HTTP_STATUS_ENTRY(BadRequest),
       HTTP_STATUS_ENTRY(Unauthorized),
       HTTP_STATUS_ENTRY(Forbidden),
+      HTTP_STATUS_ENTRY(RequestTimeout),
       HTTP_STATUS_ENTRY(NotFound),
       HTTP_STATUS_ENTRY(MethodNotAllowed),
       HTTP_STATUS_ENTRY(InternalError),