diff --git a/src/common/Reference.cpp b/src/common/Reference.cpp index e077c3d67..a54ef38e4 100644 --- a/src/common/Reference.cpp +++ b/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 diff --git a/src/common/Reference.h b/src/common/Reference.h index 295069536..3c1e9079b 100644 --- a/src/common/Reference.h +++ b/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; diff --git a/src/common/runtime.cpp b/src/common/runtime.cpp index e7ba8bd9a..6c5b5f90d 100644 --- a/src/common/runtime.cpp +++ b/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); diff --git a/src/common/runtime.h b/src/common/runtime.h index f0facdeee..6d7f8cde2 100644 --- a/src/common/runtime.h +++ b/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); } diff --git a/src/modules/love/love.cpp b/src/modules/love/love.cpp index ff2ca66fc..0271a940f 100644 --- a/src/modules/love/love.cpp +++ b/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. diff --git a/src/modules/physics/box2d/Body.cpp b/src/modules/physics/box2d/Body.cpp index 998685d49..01e7da9bd 100644 --- a/src/modules/physics/box2d/Body.cpp +++ b/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; diff --git a/src/modules/physics/box2d/Body.h b/src/modules/physics/box2d/Body.h index e399cbc92..d222a19fb 100644 --- a/src/modules/physics/box2d/Body.h +++ b/src/modules/physics/box2d/Body.h @@ -48,7 +48,7 @@ class Fixture; struct bodyudata { // Reference to arbitrary data. - Reference *ref; + Reference *ref = nullptr; }; /** diff --git a/src/modules/physics/box2d/Fixture.cpp b/src/modules/physics/box2d/Fixture.cpp index ff014355a..af33e128b 100644 --- a/src/modules/physics/box2d/Fixture.cpp +++ b/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; diff --git a/src/modules/physics/box2d/Fixture.h b/src/modules/physics/box2d/Fixture.h index f942592cd..32c362453 100644 --- a/src/modules/physics/box2d/Fixture.h +++ b/src/modules/physics/box2d/Fixture.h @@ -47,7 +47,7 @@ namespace box2d struct fixtureudata { // Reference to arbitrary data. - Reference *ref; + Reference *ref = nullptr; }; /** diff --git a/src/modules/physics/box2d/Joint.cpp b/src/modules/physics/box2d/Joint.cpp index acc76d544..a7f2d4e3a 100644 --- a/src/modules/physics/box2d/Joint.cpp +++ b/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; diff --git a/src/modules/physics/box2d/Joint.h b/src/modules/physics/box2d/Joint.h index d20bb3856..36149c9dd 100644 --- a/src/modules/physics/box2d/Joint.h +++ b/src/modules/physics/box2d/Joint.h @@ -46,7 +46,7 @@ class World; struct jointudata { // Reference to arbitrary data. - Reference *ref; + Reference *ref = nullptr; }; /** diff --git a/src/modules/physics/box2d/World.cpp b/src/modules/physics/box2d/World.cpp index 2cb973046..564d0daf6 100644 --- a/src/modules/physics/box2d/World.cpp +++ b/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; } diff --git a/src/modules/physics/box2d/World.h b/src/modules/physics/box2d/World.h index 6e4e7da7e..da35d4f7a 100644 --- a/src/modules/physics/box2d/World.h +++ b/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 diff --git a/src/modules/physics/box2d/wrap_World.cpp b/src/modules/physics/box2d/wrap_World.cpp index e88b93459..a0c86082e 100644 --- a/src/modules/physics/box2d/wrap_World.cpp +++ b/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; }