Fixed objects which store Lua callback functions to avoid attempting to use dead coroutines when deleting the reference to the callback function. Should fix crashes when coroutines are mixed with those objects.

Also improved the performance of World:rayCast and World:queryBoundingBox.

src/common/Reference.cpp

@@ -26,13 +26,13 @@ namespace love
 const char REFERENCE_TABLE_NAME[] = "love-references";
 
 Reference::Reference()
-	: L(nullptr)
+	: pinnedL(nullptr)
 	, idx(LUA_REFNIL)
 {
 }
 
 Reference::Reference(lua_State *L)
-	: L(L)
+	: pinnedL(nullptr)
 	, idx(LUA_REFNIL)
 {
 	ref(L);
@@ -46,7 +46,7 @@ Reference::~Reference()
 void Reference::ref(lua_State *L)
 {
 	unref(); // Just to be safe.
-	this->L = L;
+	pinnedL = luax_getpinnedthread(L);
 	luax_insist(L, LUA_REGISTRYINDEX, REFERENCE_TABLE_NAME);
 	lua_insert(L, -2); // Move reference table behind value.
 	idx = luaL_ref(L, -2);
@@ -57,38 +57,26 @@ void Reference::unref()
 {
 	if (idx != LUA_REFNIL)
 	{
-		luax_insist(L, LUA_REGISTRYINDEX, REFERENCE_TABLE_NAME);
-		luaL_unref(L, -1, idx);
-		lua_pop(L, 1);
+		// We use a pinned thread/coroutine for the Lua state because we know it
+		// hasn't been garbage collected and is valid, as long as the whole lua
+		// state is still open.
+		luax_insist(pinnedL, LUA_REGISTRYINDEX, REFERENCE_TABLE_NAME);
+		luaL_unref(pinnedL, -1, idx);
+		lua_pop(pinnedL, 1);
 		idx = LUA_REFNIL;
 	}
 }
 
-void Reference::push(lua_State *newL)
+void Reference::push(lua_State *L)
 {
 	if (idx != LUA_REFNIL)
 	{
-		luax_insist(newL, LUA_REGISTRYINDEX, REFERENCE_TABLE_NAME);
-		lua_rawgeti(newL, -1, idx);
-		lua_remove(newL, -2);
+		luax_insist(L, LUA_REGISTRYINDEX, REFERENCE_TABLE_NAME);
+		lua_rawgeti(L, -1, idx);
+		lua_remove(L, -2);
 	}
 	else
-		lua_pushnil(newL);
-}
-
-void Reference::push()
-{
-	push(L);
-}
-
-lua_State *Reference::getL() const
-{
-	return L;
-}
-
-void Reference::setL(lua_State *newL)
-{
-	L = newL;
+		lua_pushnil(L);
 }
 
 } // love

src/common/Reference.h

@@ -64,36 +64,17 @@ public:
 	void unref();
 
 	/**
-	 * Pushes the referred value onto the stack of a different coroutine
-	 * in the same main Lua state.
-	 * THIS SHOULD NOT BE USED FOR DIFFERENT LUA STATES (created with
-	 * luaL_newstate)! Only with different coroutines!
+	 * Pushes the referred value onto the stack of the specified Lua coroutine.
+	 * NOTE: The coroutine *must* belong to the same Lua state that was used for
+	 * Reference::ref.
 	 **/
-	void push(lua_State *newL);
-
-	/**
-	 * Pushes the referred value onto the stack.
-	 **/
-	void push();
-
-	/**
-	 * Gets the Lua state associated with this
-	 * reference.
-	 **/
-	lua_State *getL() const;
-
-	/**
-	 * Associates a new Lua state with this reference.
-	 * THIS IS DANGEROUS! It is only designed to be
-	 * used with different coroutines from the same
-	 * main Lua state!
-	 **/
-	void setL(lua_State *newL);
+	void push(lua_State *L);
 
 private:
 
-	// The Lua state in which the reference resides.
-	lua_State *L;
+	// A pinned coroutine (probably the main thread) belonging to the Lua state
+	// in which the reference resides.
+	lua_State *pinnedL;
 
 	// Index to the Lua reference.
 	int idx;

src/common/runtime.cpp

@@ -83,7 +83,7 @@ Reference *luax_refif(lua_State *L, int type)
 
 void luax_printstack(lua_State *L)
 {
-	for (int i = 1; i<=lua_gettop(L); i++)
+	for (int i = 1; i <= lua_gettop(L); i++)
 		std::cout << i << " - " << luaL_typename(L, i) << std::endl;
 }
 
@@ -590,8 +590,6 @@ int luax_insistregistry(lua_State *L, Registry r)
 {
 	switch (r)
 	{
-	case REGISTRY_GC:
-		return luax_insistlove(L, "_gc");
 	case REGISTRY_MODULES:
 		return luax_insistlove(L, "_modules");
 	case REGISTRY_OBJECTS:
@@ -605,8 +603,6 @@ int luax_getregistry(lua_State *L, Registry r)
 {
 	switch (r)
 	{
-	case REGISTRY_GC:
-		return luax_getlove(L, "_gc");
 	case REGISTRY_MODULES:
 		return luax_getlove(L, "_modules");
 	case REGISTRY_OBJECTS:
@@ -617,6 +613,39 @@ int luax_getregistry(lua_State *L, Registry r)
 	}
 }
 
+static const char *MAIN_THREAD_KEY = "_love_mainthread";
+
+lua_State *luax_insistpinnedthread(lua_State *L)
+{
+	lua_getfield(L, LUA_REGISTRYINDEX, MAIN_THREAD_KEY);
+
+	if (lua_isnoneornil(L, -1))
+	{
+		lua_pop(L, 1);
+
+		// lua_pushthread returns 1 if it's actually the main thread, but we
+		// can't actually get the real main thread if lua_pushthread doesn't
+		// return it (in Lua 5.1 at least), so we ignore that for now...
+		// We do store a strong reference to the current thread/coroutine in
+		// the registry, however.
+		lua_pushthread(L);
+		lua_pushvalue(L, -1);
+		lua_setfield(L, LUA_REGISTRYINDEX, MAIN_THREAD_KEY);
+	}
+
+	lua_State *thread = lua_tothread(L, -1);
+	lua_pop(L, 1);
+	return thread;
+}
+
+lua_State *luax_getpinnedthread(lua_State *L)
+{
+	lua_getfield(L, LUA_REGISTRYINDEX, MAIN_THREAD_KEY);
+	lua_State *thread = lua_tothread(L, -1);
+	lua_pop(L, 1);
+	return thread;
+}
+
 extern "C" int luax_typerror(lua_State *L, int narg, const char *tname)
 {
 	int argtype = lua_type(L, narg);

src/common/runtime.h

@@ -49,7 +49,6 @@ class Reference;
  **/
 enum Registry
 {
-	REGISTRY_GC,
 	REGISTRY_MODULES,
 	REGISTRY_OBJECTS
 };
@@ -390,6 +389,24 @@ int luax_insistregistry(lua_State *L, Registry r);
  **/
 int luax_getregistry(lua_State *L, Registry r);
 
+/**
+ * Gets (and pins if needed) a "pinned" Lua thread (coroutine) in the specified
+ * Lua state. This will usually be the main Lua thread, unless the first call
+ * to this function for a specific Lua state is made from within a coroutine.
+ * NOTE: This does not push anything to the stack.
+ **/
+lua_State *luax_insistpinnedthread(lua_State *L);
+
+/**
+ * Gets a "pinned" Lua thread (coroutine) in the specified Lua state. This will
+ * usually be the main Lua thread. This can be used to access global variables
+ * in a specific Lua state without needing another alive lua_State value.
+ * PRECONDITION: luax_insistpinnedthread must have been called on a lua_State
+ * value corresponding to the Lua state which will be used with this function.
+ * NOTE: This does not push anything to the stack.
+ **/
+lua_State *luax_getpinnedthread(lua_State *L);
+
 extern "C" { // Also called from luasocket
 	int luax_typerror(lua_State *L, int narg, const char *tname);
 }

src/modules/love/love.cpp

@@ -238,8 +238,10 @@ static int w_love_isVersionCompatible(lua_State *L)
 	return 1;
 }
 
-int luaopen_love(lua_State * L)
+int luaopen_love(lua_State *L)
 {
+	love::luax_insistpinnedthread(L);
+
 	love::luax_insistglobal(L, "love");
 
 	// Set version information.

src/modules/physics/box2d/Body.cpp

@@ -535,15 +535,7 @@ int Body::setUserData(lua_State *L)
 		body->SetUserData((void *) udata);
 	}
 
-	if (udata->ref != nullptr)
-	{
-		// We set the Reference's lua_State to this one before deleting it, so
-		// it unrefs using the current lua_State's stack. This is necessary
-		// if setUserData is called in a coroutine.
-		udata->ref->setL(L);
-		delete udata->ref;
-	}
-
+	delete udata->ref;
 	udata->ref = new Reference(L);
 
 	return 0;

src/modules/physics/box2d/Body.h

@@ -48,7 +48,7 @@ class Fixture;
 struct bodyudata
 {
 	// Reference to arbitrary data.
-	Reference *ref;
+	Reference *ref = nullptr;
 };
 
 /**

src/modules/physics/box2d/Fixture.cpp

@@ -227,15 +227,7 @@ int Fixture::setUserData(lua_State *L)
 {
 	love::luax_assert_argc(L, 1, 1);
 
-	if (data->ref != nullptr)
-	{
-		// We set the Reference's lua_State to this one before deleting it, so
-		// it unrefs using the current lua_State's stack. This is necessary
-		// if setUserData is called in a coroutine.
-		data->ref->setL(L);
-		delete data->ref;
-	}
-
+	delete data->ref;
 	data->ref = new Reference(L);
 
 	return 0;

src/modules/physics/box2d/Fixture.h

@@ -47,7 +47,7 @@ namespace box2d
 struct fixtureudata
 {
 	// Reference to arbitrary data.
-	Reference *ref;
+	Reference *ref = nullptr;
 };
 
 /**

src/modules/physics/box2d/Joint.cpp

@@ -193,15 +193,7 @@ int Joint::setUserData(lua_State *L)
 {
 	love::luax_assert_argc(L, 1, 1);
 
-	if (udata->ref != nullptr)
-	{
-		// We set the Reference's lua_State to this one before deleting it, so
-		// it unrefs using the current lua_State's stack. This is necessary
-		// if setUserData is called in a coroutine.
-		udata->ref->setL(L);
-		delete udata->ref;
-	}
-
+	delete udata->ref;
 	udata->ref = new Reference(L);
 
 	return 0;

src/modules/physics/box2d/Joint.h

@@ -46,7 +46,7 @@ class World;
 struct jointudata
 {
     // Reference to arbitrary data.
-    Reference *ref;
+    Reference *ref = nullptr;
 };
 
 /**

src/modules/physics/box2d/World.cpp

@@ -36,6 +36,7 @@ namespace box2d
 
 World::ContactCallback::ContactCallback()
 	: ref(nullptr)
+	, L(nullptr)
 {
 }
 
@@ -48,10 +49,9 @@ World::ContactCallback::~ContactCallback()
 void World::ContactCallback::process(b2Contact *contact, const b2ContactImpulse *impulse)
 {
 	// Process contacts.
-	if (ref != nullptr)
+	if (ref != nullptr && L != nullptr)
 	{
-		lua_State *L = ref->getL();
-		ref->push();
+		ref->push(L);
 
 		// Push first fixture.
 		{
@@ -97,6 +97,7 @@ void World::ContactCallback::process(b2Contact *contact, const b2ContactImpulse
 
 World::ContactFilter::ContactFilter()
 	: ref(nullptr)
+	, L(nullptr)
 {
 }
 
@@ -124,10 +125,9 @@ bool World::ContactFilter::process(Fixture *a, Fixture *b)
 		(filterB[1] & filterA[0]) == 0)
 		return false; // A and B aren't set to collide
 
-	if (ref != nullptr)
+	if (ref != nullptr && L != nullptr)
 	{
-		lua_State *L = ref->getL();
-		ref->push();
+		ref->push(L);
 		luax_pushtype(L, PHYSICS_FIXTURE_ID, a);
 		luax_pushtype(L, PHYSICS_FIXTURE_ID, b);
 		lua_call(L, 2, 1);
@@ -136,50 +136,51 @@ bool World::ContactFilter::process(Fixture *a, Fixture *b)
 	return true;
 }
 
-World::QueryCallback::QueryCallback()
-	: ref(nullptr)
+World::QueryCallback::QueryCallback(lua_State *L, int idx)
+	: L(L)
+	, funcidx(idx)
 {
+	luaL_checktype(L, funcidx, LUA_TFUNCTION);
 }
 
 World::QueryCallback::~QueryCallback()
 {
-	if (ref != nullptr)
-		delete ref;
 }
 
 bool World::QueryCallback::ReportFixture(b2Fixture *fixture)
 {
-	if (ref != nullptr)
+	if (L != nullptr)
 	{
-		lua_State *L = ref->getL();
-		ref->push();
+		lua_pushvalue(L, funcidx);
 		Fixture *f = (Fixture *)Memoizer::find(fixture);
 		if (!f)
 			throw love::Exception("A fixture has escaped Memoizer!");
 		luax_pushtype(L, PHYSICS_FIXTURE_ID, f);
 		lua_call(L, 1, 1);
-		return luax_toboolean(L, -1);
+		bool cont = luax_toboolean(L, -1);
+		lua_pop(L, 1);
+		return cont;
 	}
+
 	return true;
 }
 
-World::RayCastCallback::RayCastCallback()
-	: ref(nullptr)
+World::RayCastCallback::RayCastCallback(lua_State *L, int idx)
+	: L(L)
+	, funcidx(idx)
 {
+	luaL_checktype(L, funcidx, LUA_TFUNCTION);
 }
 
 World::RayCastCallback::~RayCastCallback()
 {
-	if (ref != nullptr)
-		delete ref;
 }
 
 float32 World::RayCastCallback::ReportFixture(b2Fixture *fixture, const b2Vec2 &point, const b2Vec2 &normal, float32 fraction)
 {
-	if (ref != nullptr)
+	if (L != nullptr)
 	{
-		lua_State *L = ref->getL();
-		ref->push();
+		lua_pushvalue(L, funcidx);
 		Fixture *f = (Fixture *)Memoizer::find(fixture);
 		if (!f)
 			throw love::Exception("A fixture has escaped Memoizer!");
@@ -193,8 +194,11 @@ float32 World::RayCastCallback::ReportFixture(b2Fixture *fixture, const b2Vec2 &
 		lua_call(L, 6, 1);
 		if (!lua_isnumber(L, -1))
 			luaL_error(L, "Raycast callback didn't return a number!");
-		return (float32)lua_tonumber(L, -1);
+		float32 fraction = (float32) lua_tonumber(L, -1);
+		lua_pop(L, 1);
+		return fraction;
 	}
+
 	return 0;
 }
 
@@ -345,24 +349,28 @@ int World::setCallbacks(lua_State *L)
 	{
 		lua_pushvalue(L, 1);
 		begin.ref = luax_refif(L, LUA_TFUNCTION);
+		begin.L = L;
 	}
 
 	if (nargs >= 2)
 	{
 		lua_pushvalue(L, 2);
 		end.ref = luax_refif(L, LUA_TFUNCTION);
+		end.L = L;
 	}
 
 	if (nargs >= 3)
 	{
 		lua_pushvalue(L, 3);
 		presolve.ref = luax_refif(L, LUA_TFUNCTION);
+		presolve.L = L;
 	}
 
 	if (nargs >= 4)
 	{
 		lua_pushvalue(L, 4);
 		postsolve.ref = luax_refif(L, LUA_TFUNCTION);
+		postsolve.L = L;
 	}
 
 	return 0;
@@ -377,6 +385,11 @@ int World::getCallbacks(lua_State *L)
 	return 4;
 }
 
+void World::setCallbacksL(lua_State *L)
+{
+	begin.L = end.L = presolve.L = postsolve.L = filter.L = L;
+}
+
 int World::setContactFilter(lua_State *L)
 {
 	if (!lua_isnoneornil(L, 1))
@@ -385,6 +398,7 @@ int World::setContactFilter(lua_State *L)
 	if (filter.ref)
 		delete filter.ref;
 	filter.ref = luax_refif(L, LUA_TFUNCTION);
+	filter.L = L;
 	return 0;
 }
 
@@ -519,8 +533,7 @@ int World::queryBoundingBox(lua_State *L)
 	box.lowerBound = Physics::scaleDown(b2Vec2(lx, ly));
 	box.upperBound = Physics::scaleDown(b2Vec2(ux, uy));
 	luaL_checktype(L, 5, LUA_TFUNCTION);
-	if (query.ref) delete query.ref;
-	query.ref = luax_refif(L, LUA_TFUNCTION);
+	QueryCallback query(L, 5);
 	world->QueryAABB(&query, box);
 	return 0;
 }
@@ -534,9 +547,7 @@ int World::rayCast(lua_State *L)
 	b2Vec2 v1 = Physics::scaleDown(b2Vec2(x1, y1));
 	b2Vec2 v2 = Physics::scaleDown(b2Vec2(x2, y2));
 	luaL_checktype(L, 5, LUA_TFUNCTION);
-	if (raycast.ref)
-		delete raycast.ref;
-	raycast.ref = luax_refif(L, LUA_TFUNCTION);
+	RayCastCallback raycast(L, 5);
 	world->RayCast(&raycast, v1, v2);
 	return 0;
 }

src/modules/physics/box2d/World.h

@@ -70,6 +70,7 @@ public:
 	{
 	public:
 		Reference *ref;
+		lua_State *L;
 		ContactCallback();
 		~ContactCallback();
 		void process(b2Contact *contact, const b2ContactImpulse *impulse = NULL);
@@ -79,6 +80,7 @@ public:
 	{
 	public:
 		Reference *ref;
+		lua_State *L;
 		ContactFilter();
 		~ContactFilter();
 		bool process(Fixture *a, Fixture *b);
@@ -87,19 +89,23 @@ public:
 	class QueryCallback : public b2QueryCallback
 	{
 	public:
-		Reference *ref;
-		QueryCallback();
+		QueryCallback(lua_State *L, int idx);
 		~QueryCallback();
 		virtual bool ReportFixture(b2Fixture *fixture);
+	private:
+		lua_State *L;
+		int funcidx;
 	};
 
 	class RayCastCallback : public b2RayCastCallback
 	{
 	public:
-		Reference *ref;
-		RayCastCallback();
+		RayCastCallback(lua_State *L, int idx);
 		~RayCastCallback();
 		virtual float32 ReportFixture(b2Fixture *fixture, const b2Vec2 &point, const b2Vec2 &normal, float32 fraction);
+	private:
+		lua_State *L;
+		int funcidx;
 	};
 
 	/**
@@ -157,6 +163,13 @@ public:
 	 **/
 	int getCallbacks(lua_State *L);
 
+	/**
+	 * Updates the Lua thread/coroutine used when callbacks are executed in
+	 * the update method. This should be called in the same Lua function which
+	 * calls update().
+	 **/
+	void setCallbacksL(lua_State *L);
+
 	/**
 	 * Sets the ContactFilter callback.
 	 **/
@@ -281,8 +294,6 @@ private:
 	// Contact callbacks.
 	ContactCallback begin, end, presolve, postsolve;
 	ContactFilter filter;
-	QueryCallback query;
-	RayCastCallback	raycast;
 };
 
 } // box2d

src/modules/physics/box2d/wrap_World.cpp

@@ -39,6 +39,8 @@ int w_World_update(lua_State *L)
 {
 	World *t = luax_checkworld(L, 1);
 	float dt = (float)luaL_checknumber(L, 2);
+	// Make sure the world callbacks are using the calling Lua thread.
+	t->setCallbacksL(L);
 	luax_catchexcept(L, [&](){ t->update(dt); });
 	return 0;
 }