Skip to content

Commit ac7cf06

Browse files
philterphilter
andcommitted
fix(editor): Store ScriptedContext on ScriptedObject for disposal (#12070) cb54233310
This PR hardens ScriptedContext teardown to eliminate Lua stack growth during ScriptedObject destruction and prevent teardown-time crashes. Co-authored-by: Philip Chung <philterdesign@gmail.com>
1 parent b982967 commit ac7cf06

7 files changed

Lines changed: 48 additions & 15 deletions

File tree

.rive_head

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
3afc336770a7664a2c6d0b4556d7b5ca9f8a8549
1+
cb54233310847138571ca99c90fcc9e528ffca05

include/rive/lua/rive_lua_libs.hpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1211,15 +1211,13 @@ class ScriptedContext
12111211
public:
12121212
ScriptedContext(ScriptedObject*);
12131213
ScriptedObject* scriptedObject() { return m_scriptedObject; }
1214+
void clearScriptedObject() { m_scriptedObject = nullptr; }
12141215
static constexpr uint8_t luaTag = LUA_T_COUNT + 28;
12151216
static constexpr const char* luaName = "Context";
12161217
static constexpr bool hasMetatable = true;
1217-
void dispose() { m_disposed = true; }
1218-
bool disposed() { return m_disposed; }
12191218

12201219
private:
12211220
ScriptedObject* m_scriptedObject = nullptr;
1222-
bool m_disposed = false;
12231221
};
12241222

12251223
/// Wraps [`ListenerInvocation`] for `performAction` in scripted listener

include/rive/scripted/scripted_object.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ class DataContext;
2121
class ViewModelInstanceValue;
2222
class DataBindContainer;
2323
class ScriptedProperty;
24+
class ScriptedContext;
2425

2526
class ScriptedObject : public FileAssetReferencer,
2627
public CustomPropertyContainer,
@@ -29,6 +30,7 @@ class ScriptedObject : public FileAssetReferencer,
2930
protected:
3031
int m_self = 0;
3132
int m_context = 0;
33+
ScriptedContext* m_contextPtr = nullptr;
3234
virtual void disposeScriptInputs();
3335
#ifdef WITH_RIVE_SCRIPTING
3436
#ifdef WITH_RIVE_TOOLS

src/lua/lua_scripted_context.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,9 @@ static int context_namecall(lua_State* L)
2626
if (str != nullptr)
2727
{
2828
auto scriptedContext = lua_torive<ScriptedContext>(L, 1);
29-
if (scriptedContext->disposed())
29+
if (scriptedContext->scriptedObject() == nullptr)
3030
{
31+
luaL_error(L, "context:%s() called on a disposed context", str);
3132
return 0;
3233
}
3334
switch (atom)

src/lua/rive_lua_libs.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,7 @@ int rive_lua_pcall_with_context(lua_State* state,
288288

289289
int rive_lua_pushRef(lua_State* state, int ref)
290290
{
291+
lua_checkstack(state, 1);
291292
return lua_rawgeti(state, luaRegistryIndex, ref);
292293
}
293294

src/scripted/scripted_object.cpp

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -251,11 +251,6 @@ bool ScriptedObject::scriptInit(ScriptingVM* vm)
251251
}
252252
else
253253
{
254-
// Stack: [self]
255-
lua_newrive<ScriptedContext>(L, this);
256-
// Stack: [self, ScriptedContext]
257-
m_context = lua_ref(L, -1);
258-
rive_lua_pop(L, 1);
259254
// Stack: [self]
260255
m_self = lua_ref(L, -1);
261256
#ifdef WITH_RIVE_TOOLS
@@ -273,6 +268,11 @@ bool ScriptedObject::scriptInit(ScriptingVM* vm)
273268
}
274269
if (inits())
275270
{
271+
// Stack: [self]
272+
m_contextPtr = lua_newrive<ScriptedContext>(L, this);
273+
// Stack: [self, ScriptedContext]
274+
m_context = lua_ref(L, -1);
275+
rive_lua_pop(L, 1);
276276
// Stack: [self]
277277
lua_getfield(L, -1, "init");
278278
// Stack: [self, field]
@@ -336,14 +336,18 @@ void ScriptedObject::disposeScriptInputs()
336336

337337
void ScriptedObject::disposeScriptedContext()
338338
{
339+
if (m_contextPtr != nullptr)
340+
{
341+
m_contextPtr->clearScriptedObject();
342+
m_contextPtr = nullptr;
343+
}
339344
if (m_context != 0)
340345
{
341346
lua_State* L = state();
342-
rive_lua_pushRef(L, m_context);
343-
auto scriptedContext = lua_torive<ScriptedContext>(L, -1);
344-
scriptedContext->dispose();
345-
rive_lua_pop(L, 1);
346-
lua_unref(L, m_context);
347+
if (L != nullptr)
348+
{
349+
lua_unref(L, m_context);
350+
}
347351
m_context = 0;
348352
}
349353
}

tests/unit_tests/runtime/scripting/scripting_context_test.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "rive/animation/state_machine_instance.hpp"
66
#include "rive/assets/image_asset.hpp"
77
#include "rive_file_reader.hpp"
8+
#include <string>
89

910
using namespace rive;
1011

@@ -63,6 +64,32 @@ end
6364
}
6465
}
6566

67+
TEST_CASE("Scripted Context errors when used after disposal", "[scripting]")
68+
{
69+
ScriptingTest vm(
70+
R"(
71+
function callMarkNeedsUpdate(context: Context)
72+
context:markNeedsUpdate()
73+
end
74+
)");
75+
ScriptedObjectTest scriptedObjectTest;
76+
lua_State* L = vm.state();
77+
auto top = lua_gettop(L);
78+
79+
lua_getglobal(L, "callMarkNeedsUpdate");
80+
auto scriptedContext = lua_newrive<ScriptedContext>(L, &scriptedObjectTest);
81+
scriptedContext->clearScriptedObject();
82+
int result = lua_pcall(L, 1, 0, 0);
83+
REQUIRE(result == LUA_ERRRUN);
84+
const char* error = lua_tostring(L, -1);
85+
REQUIRE(error != nullptr);
86+
CHECK(std::string(error).find(
87+
"context:markNeedsUpdate() called on a disposed context") !=
88+
std::string::npos);
89+
lua_pop(L, 1);
90+
CHECK(top == lua_gettop(L));
91+
}
92+
6693
TEST_CASE("script has access to user created view models via Data", "[silver]")
6794
{
6895
rive::SerializingFactory silver;

0 commit comments

Comments
 (0)