Added cycle detection to Variant

We accidentally enabled nested tables previously, and now it errors properly when the tables contain cycles. Before you'd get (or at least I got) a nice lua stack overflow error.

--HG--
branch : minor

changes.txt

@@ -53,6 +53,7 @@ Released: N/A
   * Added a variant of Mesh:setVertexMap which accepts a Data object.
   * Added love.window.updateMode.
   * Added the ability to prevent love from creating a stencil buffer for the window.
+  * Added cycle detection to Variant's table serialization, cycles now cause an error, rather than a stack overflow.
 
   * Changed all color values to be in the range 0-1, rather than 0-255.
   * Changed high-dpi functionality to require much less code (often none at all) for things to appear at the correct sizes and positions.

src/common/Variant.cpp

@@ -18,6 +18,8 @@
  * 3. This notice may not be removed or altered from any source distribution.
  **/
 
+#include <memory>
+
 #include "Variant.h"
 #include "common/StringMap.h"
 
@@ -162,7 +164,7 @@ Variant &Variant::operator = (const Variant &v)
 	return *this;
 }
 
-Variant Variant::fromLua(lua_State *L, int n, bool allowTables)
+Variant Variant::fromLua(lua_State *L, int n, std::set<const void*> *tableSet)
 {
 	size_t len;
 	const char *str;
@@ -186,11 +188,23 @@ Variant Variant::fromLua(lua_State *L, int n, bool allowTables)
 	case LUA_TNIL:
 		return Variant();
 	case LUA_TTABLE:
-		if (allowTables)
 		{
 			bool success = true;
+			std::unique_ptr<std::set<const void*>> tableSetPtr;
 			std::vector<std::pair<Variant, Variant>> *table = new std::vector<std::pair<Variant, Variant>>();
 
+			// If we had no tables argument, allocate one now, and store it in our unique_ptr
+			if (tableSet == nullptr)
+				tableSetPtr.reset(tableSet = new std::set<const void*>);
+
+			// Now make sure this table wasn't already serialised
+			{
+				const void *table = lua_topointer(L, n);
+				auto result = tableSet->insert(table);
+				if (!result.second) // insertion failed
+					throw love::Exception("Cycle detected in table");
+			}
+
 			size_t len = luax_objlen(L, -1);
 			if (len > 0)
 				table->reserve(len);
@@ -199,7 +213,7 @@ Variant Variant::fromLua(lua_State *L, int n, bool allowTables)
 
 			while (lua_next(L, n))
 			{
-				table->emplace_back(fromLua(L, -2), fromLua(L, -1));
+				table->emplace_back(fromLua(L, -2, tableSet), fromLua(L, -1, tableSet));
 				lua_pop(L, 1);
 
 				const auto &p = table->back();

src/common/Variant.h

@@ -27,6 +27,7 @@
 
 #include <cstring>
 #include <vector>
+#include <set>
 
 namespace love
 {
@@ -63,7 +64,7 @@ public:
 
 	Type getType() const { return type; }
 
-	static Variant fromLua(lua_State *L, int n, bool allowTables = true);
+	static Variant fromLua(lua_State *L, int n, std::set<const void*> *tableSet = nullptr);
 	void toLua(lua_State *L) const;
 
 private:

src/modules/event/Event.cpp

@@ -63,7 +63,9 @@ Message *Message::fromLua(lua_State *L, int n)
 		if (lua_isnoneornil(L, n+i))
 			break;
 
-		vargs.push_back(Variant::fromLua(L, n+i));
+		luax_catchexcept(L, [&]() {
+			vargs.push_back(Variant::fromLua(L, n+i));
+		});
 
 		if (vargs.back().getType() == Variant::UNKNOWN)
 		{

src/modules/event/wrap_Event.cpp

@@ -93,10 +93,12 @@ int w_clear(lua_State *L)
 
 int w_quit(lua_State *L)
 {
-	std::vector<Variant> args = {Variant::fromLua(L, 1)};
+	luax_catchexcept(L, [&]() {
+		std::vector<Variant> args = {Variant::fromLua(L, 1)};
 
-	StrongRef<Message> m(new Message("quit", args), Acquire::NORETAIN);
-	instance()->push(m);
+		StrongRef<Message> m(new Message("quit", args), Acquire::NORETAIN);
+		instance()->push(m);
+	});
 
 	luax_pushboolean(L, true);
 	return 1;

src/modules/thread/wrap_Channel.cpp

@@ -33,21 +33,25 @@ Channel *luax_checkchannel(lua_State *L, int idx)
 int w_Channel_push(lua_State *L)
 {
 	Channel *c = luax_checkchannel(L, 1);
-	Variant var = Variant::fromLua(L, 2);
-	if (var.getType() == Variant::UNKNOWN)
-		return luaL_argerror(L, 2, "boolean, number, string, love type, or flat table expected");
-	uint64 id = c->push(var);
-	lua_pushnumber(L, (lua_Number) id);
+	luax_catchexcept(L, [&]() {
+		Variant var = Variant::fromLua(L, 2);
+		if (var.getType() == Variant::UNKNOWN)
+			luaL_argerror(L, 2, "boolean, number, string, love type, or table expected");
+		uint64 id = c->push(var);
+		lua_pushnumber(L, (lua_Number) id);
+	});
 	return 1;
 }
 
 int w_Channel_supply(lua_State *L)
 {
 	Channel *c = luax_checkchannel(L, 1);
-	Variant var = Variant::fromLua(L, 2);
-	if (var.getType() == Variant::UNKNOWN)
-		return luaL_argerror(L, 2, "boolean, number, string, love type, or flat table expected");
-	c->supply(var);
+	luax_catchexcept(L, [&]() {
+		Variant var = Variant::fromLua(L, 2);
+		if (var.getType() == Variant::UNKNOWN)
+			luaL_argerror(L, 2, "boolean, number, string, love type, or table expected");
+		c->supply(var);
+	});
 	return 0;
 }
 

src/modules/thread/wrap_LuaThread.cpp

@@ -38,7 +38,9 @@ int w_Thread_start(lua_State *L)
 
 	for (int i = 0; i < nargs; ++i)
 	{
-		args.push_back(Variant::fromLua(L, i+2));
+		luax_catchexcept(L, [&]() {
+			args.push_back(Variant::fromLua(L, i+2));
+		});
 
 		if (args.back().getType() == Variant::UNKNOWN)
 		{