Fix Issue #1273

Added Lua index registry clear routine to `destroy` method of classes that use it.
If this is left to GC, the abandoned userdata would stay in memory for one full extra cycle.

Prettified the code slightly. Clarified some comments. Refactored `Fixture` class `udata` name.
Removed Reference class include from `Shape` (it doesn't use it).

src/common/Reference.cpp

@@ -45,7 +45,7 @@ Reference::~Reference()
 
 void Reference::ref(lua_State *L)
 {
-	unref(); // Just to be safe.
+	unref(); // Previously created reference needs to be cleared
 	pinnedL = luax_getpinnedthread(L);
 	luax_insist(L, LUA_REGISTRYINDEX, REFERENCE_TABLE_NAME);
 	lua_insert(L, -2); // Move reference table behind value.

src/common/runtime.cpp

@@ -83,7 +83,7 @@ Reference *luax_refif(lua_State *L, int type)
 	// Create a reference only if the test succeeds.
 	if (lua_type(L, -1) == type)
 		r = new Reference(L);
-	else // Pop the value even if it fails (but also if it succeeds).
+	else // Pop the value manually if it fails (done by Reference if it succeeds).
 		lua_pop(L, 1);
 
 	return r;

src/modules/physics/box2d/Body.cpp

@@ -67,8 +67,12 @@ Body::Body(b2Body *b)
 
 Body::~Body()
 {
-	if (udata != nullptr)
+	if (!udata)
+		return;
+
+	if (udata->ref)
 		delete udata->ref;
+
 	delete udata;
 }
 
@@ -521,6 +525,10 @@ void Body::destroy()
 	Memoizer::remove(body);
 	body = NULL;
 
+	// Remove userdata reference to avoid it sticking around after GC
+	if (udata && udata->ref)
+		udata->ref->unref();
+
 	// Box2D body destroyed. Release its reference to the love Body.
 	this->release();
 }
@@ -535,8 +543,10 @@ int Body::setUserData(lua_State *L)
 		body->SetUserData((void *) udata);
 	}
 
-	delete udata->ref;
-	udata->ref = new Reference(L);
+	if(!udata->ref)
+		udata->ref = new Reference();
+
+	udata->ref->ref(L);
 
 	return 0;
 }

src/modules/physics/box2d/Body.h

@@ -69,7 +69,7 @@ public:
 	friend class Shape;
 	friend class Fixture;
 
-	// The Box2D body. (Should not be public?)
+	// Public because joints et al ask for b2body
 	b2Body *body;
 
 	/**

src/modules/physics/box2d/Fixture.cpp

@@ -41,11 +41,11 @@ Fixture::Fixture(Body *body, Shape *shape, float density)
 	: body(body)
 	, fixture(nullptr)
 {
-	data = new fixtureudata();
-	data->ref = nullptr;
+	udata = new fixtureudata();
+	udata->ref = nullptr;
 	b2FixtureDef def;
 	def.shape = shape->shape;
-	def.userData = (void *)data;
+	def.userData = (void *)udata;
 	def.density = density;
 	fixture = body->body->CreateFixture(&def);
 	this->retain();
@@ -55,7 +55,7 @@ Fixture::Fixture(Body *body, Shape *shape, float density)
 Fixture::Fixture(b2Fixture *f)
 	: fixture(f)
 {
-	data = (fixtureudata *)f->GetUserData();
+	udata = (fixtureudata *)f->GetUserData();
 	body = (Body *)Memoizer::find(f->GetBody());
 	if (!body)
 		body = new Body(f->GetBody());
@@ -65,10 +65,13 @@ Fixture::Fixture(b2Fixture *f)
 
 Fixture::~Fixture()
 {
-	if (data != nullptr)
-		delete data->ref;
+	if (!udata)
+		return;
+
+	if (udata->ref)
+		delete udata->ref;
 
-	delete data;
+	delete udata;
 }
 
 Shape::Type Fixture::getType() const
@@ -239,16 +242,24 @@ int Fixture::setUserData(lua_State *L)
 {
 	love::luax_assert_argc(L, 1, 1);
 
-	delete data->ref;
-	data->ref = new Reference(L);
+	if (udata == nullptr)
+	{
+		udata = new fixtureudata();
+		fixture->SetUserData((void *) udata);
+	}
+
+	if(!udata->ref)
+		udata->ref = new Reference();
+
+	udata->ref->ref(L);
 
 	return 0;
 }
 
 int Fixture::getUserData(lua_State *L)
 {
-	if (data->ref != nullptr)
-		data->ref->push(L);
+	if (udata->ref != nullptr)
+		udata->ref->push(L);
 	else
 		lua_pushnil(L);
 
@@ -321,6 +332,10 @@ void Fixture::destroy(bool implicit)
 	Memoizer::remove(fixture);
 	fixture = nullptr;
 
+	// Remove userdata reference to avoid it sticking around after GC
+	if (udata && udata->ref)
+		udata->ref->unref();
+
 	// Box2D fixture destroyed. Release its reference to the love Fixture.
 	this->release();
 }

src/modules/physics/box2d/Fixture.h

@@ -211,7 +211,7 @@ public:
 protected:
 
 	Body *body;
-	fixtureudata *data;
+	fixtureudata *udata;
 	b2Fixture *fixture;
 };
 

src/modules/physics/box2d/Joint.cpp

@@ -61,8 +61,12 @@ Joint::Joint(Body *body1, Body *body2)
 
 Joint::~Joint()
 {
-	if (udata != nullptr)
+	if (!udata)
+		return;
+
+	if (udata->ref)
 		delete udata->ref;
+
 	delete udata;
 }
 
@@ -175,6 +179,11 @@ void Joint::destroyJoint(bool implicit)
 		world->world->DestroyJoint(joint);
 	Memoizer::remove(joint);
 	joint = NULL;
+
+	// Remove userdata reference to avoid it sticking around after GC
+	if (udata && udata->ref)
+		udata->ref->unref();
+
 	// Release the reference of the Box2D joint.
 	this->release();
 }
@@ -193,8 +202,16 @@ int Joint::setUserData(lua_State *L)
 {
 	love::luax_assert_argc(L, 1, 1);
 
-	delete udata->ref;
-	udata->ref = new Reference(L);
+	if (udata == nullptr)
+	{
+		udata = new jointudata();
+		joint->SetUserData((void *) udata);
+	}
+
+	if(!udata->ref)
+		udata->ref = new Reference();
+
+	udata->ref->ref(L);
 
 	return 0;
 }

src/modules/physics/box2d/Shape.h

@@ -24,7 +24,6 @@
 // LOVE
 #include "physics/Shape.h"
 #include "physics/box2d/Body.h"
-#include "common/Reference.h"
 
 // Box2D
 #include <Box2D/Box2D.h>

src/modules/physics/box2d/World.cpp

@@ -577,6 +577,14 @@ void World::destroy()
 
 	world->DestroyBody(groundBody);
 	Memoizer::remove(world);
+
+	// Remove userdata reference to avoid it sticking around after GC
+	if (begin.ref)     begin.ref->unref();
+	if (end.ref)       end.ref->unref();
+	if (presolve.ref)  presolve.ref->unref();
+	if (postsolve.ref) postsolve.ref->unref();
+	if (filter.ref)    filter.ref->unref();
+
 	delete world;
 	world = nullptr;
 }