🛠 Guard against environment over-pushing

include/sol/environment.hpp

@@ -62,7 +62,7 @@ namespace sol {
 			constructor_handler handler {};
 			stack::check<env_key_t>(this->lua_state(), -1, handler);
 #endif // Safety
-			lua_pop(this->lua_state(), 2);
+			lua_pop(this->lua_state(), 1);
 		}
 		template <bool b>
 		basic_environment(env_key_t, const basic_reference<b>& extraction_target)
@@ -71,7 +71,7 @@ namespace sol {
 			constructor_handler handler {};
 			stack::check<env_key_t>(this->lua_state(), -1, handler);
 #endif // Safety
-			lua_pop(this->lua_state(), 2);
+			lua_pop(this->lua_state(), 1);
 		}
 		basic_environment(lua_State* L, int index = -1) : base_t(detail::no_safety, L, index) {
 #if SOL_IS_ON(SOL_SAFE_REFERENCES)
@@ -136,11 +136,13 @@ namespace sol {
 					const char* success = lua_setupvalue(L, target_index, 1);
 					if (success == nullptr) {
 						// left things alone on the stack, pop them off
-						lua_pop(L, 1);
+						lua_pop(L, 2);
 						return false;
 					}
+					lua_pop(L, 1);
 					return true;
 				}
+				lua_pop(L, 1);
 				return false;
 			}
 			else {
@@ -152,6 +154,7 @@ namespace sol {
 					}
 					string_view upvalue_name(maybe_upvalue_name);
 					if (upvalue_name == "_ENV") {
+						lua_pop(L, 1);
 						this->push();
 						const char* success = lua_setupvalue(L, target_index, upvalue_index);
 						if (success == nullptr) {

include/sol/stack_push.hpp

@@ -30,6 +30,7 @@
 #include <sol/usertype_traits.hpp>
 #include <sol/policies.hpp>
 #include <sol/unicode.hpp>
+#include <sol/assert.hpp>
 
 #include <memory>
 #include <type_traits>
@@ -41,6 +42,8 @@
 #include <variant>
 #endif // Can use variant
 
+#include <sol/debug.hpp>
+
 namespace sol { namespace stack {
 	namespace stack_detail {
 		template <typename T>
@@ -141,7 +144,11 @@ namespace sol { namespace stack {
 	int push_environment_of(const T& target) {
 		lua_State* target_L = target.lua_state();
 		int target_index = absolute_index(target_L, -target.push());
-		return push_environment_of(target_L, target_index);
+		int env_count = push_environment_of(target_L, target_index);
+		sol_c_assert(env_count == 1);
+		lua_rotate(target_L, target_index, 1);
+		lua_pop(target_L, 1);
+		return env_count;
 	}
 
 	template <typename T>
@@ -316,7 +323,7 @@ namespace sol { namespace stack {
 				if (static_cast<T>(llround(static_cast<lua_Number>(value))) != value) {
 #if SOL_IS_OFF(SOL_EXCEPTIONS)
 					// Is this really worth it?
-					assert(false && "integer value will be misrepresented in lua");
+					sol_m_assert(false, "integer value will be misrepresented in lua");
 					lua_pushinteger(L, static_cast<lua_Integer>(value));
 					return 1;
 #else

tests/environment/source/get.cpp

@@ -57,6 +57,7 @@ inline namespace sol2_tests_environments_get {
 			sol::state& lua = *plua;
 			sol::environment& env_f = *penv_f;
 			sol::stack_guard luasg(lua);
+			{
 				sol::environment target_env(sol::env_key, target);
 				int test_env_f = env_f["test"];
 				int test_target_env = target_env["test"];
@@ -64,6 +65,7 @@ inline namespace sol2_tests_environments_get {
 				REQUIRE(test_env_f == 31);
 				REQUIRE(env_f == target_env);
 			}
+		}
 	};
 
 	struct check_h_env {
@@ -89,20 +91,27 @@ TEST_CASE("environments/get", "Envronments can be taken out of things like Lua f
 
 	lua.open_libraries(sol::lib::base);
 
+	{
 		auto result1 = lua.safe_script("f = function() return test end", sol::script_pass_on_error);
 		REQUIRE(result1.valid());
+	}
 	sol::function f = lua["f"];
 
 	sol::environment env_f(lua, sol::create);
 	env_f["test"] = 31;
+	{
+		sol::stack_guard luasgf(lua);
 		bool env_f_was_set = sol::set_environment(env_f, f);
 		REQUIRE(env_f_was_set);
+	}
 
 	int result = f();
 	REQUIRE(result == 31);
 
+	{
 		auto result2 = lua.safe_script("g = function() test = 5 end", sol::script_pass_on_error);
 		REQUIRE(result2.valid());
+	}
 	sol::function g = lua["g"];
 	sol::environment env_g(lua, sol::create);
 	bool env_g_was_set = env_g.set_on(g);
@@ -116,13 +125,16 @@ TEST_CASE("environments/get", "Envronments can be taken out of things like Lua f
 	sol::object global_test = lua["test"];
 	REQUIRE(!global_test.valid());
 
+	{
 		auto result3 = lua.safe_script("h = function() end", sol::script_pass_on_error);
 		REQUIRE(result3.valid());
+	}
 
 	lua.set_function("check_f_env", check_f_env(lua, env_f));
 	lua.set_function("check_g_env", check_g_env(lua, env_g));
 	lua.set_function("check_h_env", check_h_env(lua));
 
+	{
 		auto checkf = lua.safe_script("check_f_env(f)", sol::script_pass_on_error);
 		REQUIRE(checkf.valid());
 		auto checkg = lua.safe_script("check_g_env(g)", sol::script_pass_on_error);
@@ -130,3 +142,4 @@ TEST_CASE("environments/get", "Envronments can be taken out of things like Lua f
 		auto checkh = lua.safe_script("check_h_env(h)", sol::script_pass_on_error);
 		REQUIRE(checkh.valid());
 	}
+}

tests/environment/source/this_environment.cpp

@@ -44,15 +44,21 @@ TEST_CASE("environments/this_environment", "test various situations of pulling o
 	lua["x"] = 5;
 	e["x"] = 20;
 	SECTION("from Lua script") {
+		int value = 0;
+		{
 			auto result1 = lua.safe_script(code, e, sol::script_pass_on_error);
 			REQUIRE(result1.valid());
-		int value = result1;
+			value = result1;
+		}
 		REQUIRE(value == 30);
 	}
 	SECTION("from C++") {
 		sol::function f = lua["f"];
+		{
+			sol::stack_guard luasg_env(lua);
 			bool env_set = e.set_on(f);
 			REQUIRE(env_set);
+		}
 		int value = f(10);
 		REQUIRE(value == 30);
 	}