lua: fix a bug where the lifetime of userdata will result in crash (#…

…35063)

Commit Message: lua: fix a bug where the lifetime of userdata will
result in crash
Additional Description:

The RouteHandleWrapper will be desotryed when the lua doing the gc. And
the embedded HeaderMapRef will call the reset() when it's destructed and
will refer the invalid lua_State pointer and finally result in a crash.

Risk Level: low.
Testing: unit.
Docs Changes: n/a.
Release Notes: added.
Platform Specific Features: n/a.

---------

Signed-off-by: wbpcode <wbphub@live.com>
Co-authored-by: wbpcode <wbphub@live.com>

changelogs/current.yaml

@@ -237,6 +237,9 @@ bug_fixes:
 - area: datadog
   change: |
     Bumped the version of datadog to resolve a crashing bug in earlier versions of the library.
+- area: lua
+  change: |
+    Fixed a bug where the user data will reference a dangling pointer to the Lua state and cause a crash.
 
 removed_config_or_runtime:
 # *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`

source/extensions/router/cluster_specifiers/lua/lua_cluster_specifier.h

@@ -25,6 +25,8 @@ class PerLuaCodeSetup : Logger::Loggable<Logger::Id::lua> {
 
   int clusterFunctionRef() { return lua_state_.getGlobalRef(cluster_function_slot_); }
 
+  void runtimeGC() { lua_state_.runtimeGC(); }
+
 private:
   uint64_t cluster_function_slot_{};
 
@@ -58,6 +60,11 @@ class RouteHandleWrapper : public Filters::Common::Lua::BaseLuaObject<RouteHandl
 
   static ExportedFunctions exportedFunctions() { return {{"headers", static_luaHeaders}}; }
 
+  // All embedded references should be reset when the object is marked dead. This is to ensure that
+  // we won't do the resetting in the destructor, which may be called after the referenced
+  // coroutine's lua_State is closed. And if that happens, the resetting will cause a crash.
+  void onMarkDead() override { headers_wrapper_.reset(); }
+
 private:
   /**
    * @return a handle to the headers.

test/extensions/router/cluster_specifiers/lua/lua_cluster_specifier_test.cc

@@ -80,12 +80,16 @@ TEST_F(LuaClusterSpecifierPluginTest, NormalLuaCode) {
     Http::TestRequestHeaderMapImpl headers{{":path", "/"}, {"header_key", "fake"}};
     auto route = plugin_->route(mock_route, headers);
     EXPECT_EQ("fake_service", route->routeEntry()->clusterName());
+    // Force the runtime to gc and destroy all the userdata.
+    config_->perLuaCodeSetup()->runtimeGC();
   }
 
   {
     Http::TestRequestHeaderMapImpl headers{{":path", "/"}, {"header_key", "header_value"}};
     auto route = plugin_->route(mock_route, headers);
     EXPECT_EQ("web_service", route->routeEntry()->clusterName());
+    // Force the runtime to gc and destroy all the userdata.
+    config_->perLuaCodeSetup()->runtimeGC();
   }
 }
 
@@ -98,12 +102,16 @@ TEST_F(LuaClusterSpecifierPluginTest, ErrorLuaCode) {
     Http::TestRequestHeaderMapImpl headers{{":path", "/"}, {"header_key", "fake"}};
     auto route = plugin_->route(mock_route, headers);
     EXPECT_EQ("default_service", route->routeEntry()->clusterName());
+    // Force the runtime to gc and destroy all the userdata.
+    config_->perLuaCodeSetup()->runtimeGC();
   }
 
   {
     Http::TestRequestHeaderMapImpl headers{{":path", "/"}, {"header_key", "header_value"}};
     auto route = plugin_->route(mock_route, headers);
     EXPECT_EQ("default_service", route->routeEntry()->clusterName());
+    // Force the runtime to gc and destroy all the userdata.
+    config_->perLuaCodeSetup()->runtimeGC();
   }
 }
 
@@ -116,12 +124,16 @@ TEST_F(LuaClusterSpecifierPluginTest, ReturnTypeNotStringLuaCode) {
     Http::TestRequestHeaderMapImpl headers{{":path", "/"}, {"header_key", "fake"}};
     auto route = plugin_->route(mock_route, headers);
     EXPECT_EQ("fake_service", route->routeEntry()->clusterName());
+    // Force the runtime to gc and destroy all the userdata.
+    config_->perLuaCodeSetup()->runtimeGC();
   }
 
   {
     Http::TestRequestHeaderMapImpl headers{{":path", "/"}, {"header_key", "header_value"}};
     auto route = plugin_->route(mock_route, headers);
     EXPECT_EQ("default_service", route->routeEntry()->clusterName());
+    // Force the runtime to gc and destroy all the userdata.
+    config_->perLuaCodeSetup()->runtimeGC();
   }
 }