Skip to content

Commit 90cbe04

Browse files
committed
Fix Lua VMI leak: null C++ refs on orphaned sub-artboard userdata
Sub-artboard scripts store m_context, m_dataRef, m_propertyRefs as independent registry refs via lua_ref(). When the parent artboard is destroyed, only top-level scripts' refs are unreffed — the sub-artboard refs remain as GC roots, keeping C++ objects alive permanently. Track ScriptReffedArtboard and ScriptedViewModel instances in linked lists and null their C++ rcp<> pointers during artboard destruction.
1 parent 4cbab11 commit 90cbe04

File tree

8 files changed

+345
-13
lines changed

8 files changed

+345
-13
lines changed

include/rive/lua/rive_lua_libs.hpp

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -572,7 +572,8 @@ class ScriptedRenderer
572572
class ScriptReffedArtboard : public RefCnt<ScriptReffedArtboard>
573573
{
574574
public:
575-
ScriptReffedArtboard(rcp<File> file,
575+
ScriptReffedArtboard(lua_State* L,
576+
rcp<File> file,
576577
std::unique_ptr<ArtboardInstance>&& artboardInstance,
577578
rcp<ViewModelInstance> viewModelInstance,
578579
rcp<DataContext> parentDataContext);
@@ -582,12 +583,18 @@ class ScriptReffedArtboard : public RefCnt<ScriptReffedArtboard>
582583
Artboard* artboard();
583584
StateMachineInstance* stateMachine();
584585
rcp<ViewModelInstance> viewModelInstance() { return m_viewModelInstance; }
586+
void releaseReferences();
587+
static void releaseAll(lua_State* state);
585588

586589
private:
587590
rcp<File> m_file;
588591
std::unique_ptr<ArtboardInstance> m_artboard;
589592
std::unique_ptr<StateMachineInstance> m_stateMachine;
590593
rcp<ViewModelInstance> m_viewModelInstance;
594+
lua_State* m_luaState = nullptr;
595+
ScriptReffedArtboard* m_trackNext = nullptr;
596+
ScriptReffedArtboard* m_trackPrev = nullptr;
597+
static ScriptReffedArtboard* s_head;
591598
};
592599

593600
class ScriptedArtboard
@@ -630,7 +637,6 @@ class ScriptedArtboard
630637
private:
631638
lua_State* m_state = nullptr;
632639
rcp<ScriptReffedArtboard> m_scriptReffedArtboard = nullptr;
633-
rcp<DataContext> m_dataContext = nullptr;
634640
int m_dataRef = 0;
635641
};
636642

@@ -704,11 +710,17 @@ class ScriptedViewModel
704710
return m_viewModelInstance;
705711
}
706712

713+
void releaseReferences();
714+
static void releaseAll(lua_State* state);
715+
707716
private:
708717
lua_State* m_state;
709718
rcp<ViewModel> m_viewModel;
710719
rcp<ViewModelInstance> m_viewModelInstance;
711720
std::unordered_map<std::string, int> m_propertyRefs;
721+
ScriptedViewModel* m_trackNext = nullptr;
722+
ScriptedViewModel* m_trackPrev = nullptr;
723+
static ScriptedViewModel* s_head;
712724
};
713725

714726
class ScriptedPropertyViewModel : public ScriptedProperty

include/rive/scripted/scripted_object.hpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,9 @@ class ScriptedObject : public FileAssetReferencer,
5959
lua_State* state() const { return m_vm ? m_vm->state() : nullptr; }
6060
#endif
6161
void scriptDispose();
62+
#ifdef WITH_RIVE_SCRIPTING
63+
static void collectLuaGarbage(lua_State* state);
64+
#endif
6265
virtual bool addScriptedDirt(ComponentDirt value, bool recurse = false) = 0;
6366
void setAsset(rcp<FileAsset> asset) override;
6467
static ScriptedObject* from(Core* object);

src/artboard.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,13 @@ Artboard::~Artboard()
141141
return vmObjects.count(object) != 0;
142142
};
143143

144+
#ifdef WITH_RIVE_SCRIPTING
145+
lua_State* luaState = nullptr;
146+
if (!m_ScriptedObjects.empty())
147+
{
148+
luaState = m_ScriptedObjects[0]->state();
149+
}
150+
#endif
144151
// Second pass: delete non-VM objects.
145152
for (auto object : m_Objects)
146153
{
@@ -154,6 +161,9 @@ Artboard::~Artboard()
154161
}
155162
delete object;
156163
}
164+
#ifdef WITH_RIVE_SCRIPTING
165+
ScriptedObject::collectLuaGarbage(luaState);
166+
#endif
157167
for (auto object : m_invalidObjects)
158168
{
159169
if (object == nullptr)

src/lua/lua_artboards.cpp

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,18 @@
1515

1616
using namespace rive;
1717

18+
ScriptReffedArtboard* ScriptReffedArtboard::s_head = nullptr;
19+
1820
ScriptReffedArtboard::ScriptReffedArtboard(
21+
lua_State* L,
1922
rcp<File> file,
2023
std::unique_ptr<ArtboardInstance>&& artboardInstance,
2124
rcp<ViewModelInstance> viewModelInstance,
2225
rcp<DataContext> parentDataContext) :
2326
m_file(file),
2427
m_artboard(std::move(artboardInstance)),
25-
m_stateMachine(m_artboard->defaultStateMachine())
28+
m_stateMachine(m_artboard->defaultStateMachine()),
29+
m_luaState(L)
2630
{
2731
if (viewModelInstance)
2832
{
@@ -45,18 +49,58 @@ ScriptReffedArtboard::ScriptReffedArtboard(
4549
m_stateMachine->bindViewModelInstance(m_viewModelInstance);
4650
}
4751
}
52+
// Track this instance for cleanup during artboard destruction
53+
m_trackNext = s_head;
54+
m_trackPrev = nullptr;
55+
if (s_head)
56+
{
57+
s_head->m_trackPrev = this;
58+
}
59+
s_head = this;
4860
}
4961

5062
ScriptReffedArtboard::~ScriptReffedArtboard()
5163
{
52-
// Make sure state machine is deleted before artboard since
53-
// StateMachineInstance destructor accesses the artboard.
64+
// Remove from tracking list
65+
if (m_trackPrev)
66+
{
67+
m_trackPrev->m_trackNext = m_trackNext;
68+
}
69+
else if (s_head == this)
70+
{
71+
s_head = m_trackNext;
72+
}
73+
if (m_trackNext)
74+
{
75+
m_trackNext->m_trackPrev = m_trackPrev;
76+
}
77+
releaseReferences();
78+
}
79+
80+
void ScriptReffedArtboard::releaseReferences()
81+
{
82+
// StateMachine must be deleted before artboard since its destructor
83+
// accesses the artboard.
5484
m_stateMachine = nullptr;
55-
// Make sure artboard is deleted before file.
5685
m_artboard = nullptr;
86+
m_viewModelInstance = nullptr;
5787
m_file = nullptr;
5888
}
5989

90+
void ScriptReffedArtboard::releaseAll(lua_State* state)
91+
{
92+
ScriptReffedArtboard* current = s_head;
93+
while (current)
94+
{
95+
ScriptReffedArtboard* next = current->m_trackNext;
96+
if (current->m_luaState == state)
97+
{
98+
current->releaseReferences();
99+
}
100+
current = next;
101+
}
102+
}
103+
60104
rive::rcp<rive::File> ScriptReffedArtboard::file() { return m_file; }
61105

62106
Artboard* ScriptReffedArtboard::artboard() { return m_artboard.get(); }
@@ -264,7 +308,7 @@ int ScriptedArtboard::instance(lua_State* L,
264308
m_scriptReffedArtboard->file(),
265309
std::move(artboardInstance),
266310
viewModelInstance,
267-
m_dataContext);
311+
artboard()->dataContext());
268312
return 1;
269313
}
270314

@@ -384,11 +428,11 @@ ScriptedArtboard::ScriptedArtboard(
384428
rcp<DataContext> dataContext) :
385429
m_state(L),
386430
m_scriptReffedArtboard(
387-
make_rcp<ScriptReffedArtboard>(file,
431+
make_rcp<ScriptReffedArtboard>(L,
432+
file,
388433
std::move(artboardInstance),
389434
viewModelInstance,
390-
dataContext)),
391-
m_dataContext(dataContext)
435+
dataContext))
392436
{}
393437

394438
ScriptedArtboard::~ScriptedArtboard()

src/lua/lua_properties.cpp

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,18 +245,61 @@ void ScriptedPropertyViewModel::setValue(ScriptedViewModel* scriptedViewModel)
245245
}
246246
}
247247

248+
ScriptedViewModel* ScriptedViewModel::s_head = nullptr;
249+
248250
ScriptedViewModel::ScriptedViewModel(lua_State* L,
249251
rcp<ViewModel> viewModel,
250252
rcp<ViewModelInstance> viewModelInstance) :
251253
m_state(L), m_viewModel(viewModel), m_viewModelInstance(viewModelInstance)
252-
{}
254+
{
255+
m_trackNext = s_head;
256+
m_trackPrev = nullptr;
257+
if (s_head)
258+
{
259+
s_head->m_trackPrev = this;
260+
}
261+
s_head = this;
262+
}
253263

254264
ScriptedViewModel::~ScriptedViewModel()
255265
{
266+
if (m_trackPrev)
267+
{
268+
m_trackPrev->m_trackNext = m_trackNext;
269+
}
270+
else if (s_head == this)
271+
{
272+
s_head = m_trackNext;
273+
}
274+
if (m_trackNext)
275+
{
276+
m_trackNext->m_trackPrev = m_trackPrev;
277+
}
256278
for (auto itr : m_propertyRefs)
257279
{
258280
lua_unref(m_state, itr.second);
259281
}
282+
releaseReferences();
283+
}
284+
285+
void ScriptedViewModel::releaseReferences()
286+
{
287+
m_viewModel = nullptr;
288+
m_viewModelInstance = nullptr;
289+
}
290+
291+
void ScriptedViewModel::releaseAll(lua_State* state)
292+
{
293+
ScriptedViewModel* current = s_head;
294+
while (current)
295+
{
296+
ScriptedViewModel* next = current->m_trackNext;
297+
if (current->m_state == state)
298+
{
299+
current->releaseReferences();
300+
}
301+
current = next;
302+
}
260303
}
261304

262305
int ScriptedViewModel::instance(lua_State* L)

src/scripted/scripted_object.cpp

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -355,14 +355,27 @@ void ScriptedObject::scriptDispose()
355355
lua_unref(L, m_self);
356356
disposeScriptedContext();
357357
#ifdef TESTING
358-
// Force GC to collect any ScriptedArtboard instances created via
359-
// instance()
360358
lua_gc(L, LUA_GCCOLLECT, 0);
361359
#endif
362360
}
363361
m_vm = nullptr;
364362
m_self = 0;
365363
}
364+
365+
void ScriptedObject::collectLuaGarbage(lua_State* state)
366+
{
367+
if (state != nullptr)
368+
{
369+
// Null C++ pointers on all ScriptReffedArtboard and ScriptedViewModel
370+
// instances for this lua_State. Sub-artboard scripts store orphaned
371+
// registry refs (m_context, m_dataRef, m_propertyRefs) that keep C++
372+
// objects alive. Rather than walk the registry, we null the C++ side
373+
// directly — the Lua userdata become harmless empty shells.
374+
ScriptReffedArtboard::releaseAll(state);
375+
ScriptedViewModel::releaseAll(state);
376+
lua_gc(state, LUA_GCCOLLECT, 0);
377+
}
378+
}
366379
#else
367380
void ScriptedObject::setArtboardInput(std::string name, Artboard* artboard) {}
368381

tests/unit_tests/assets/blinko.riv

2.56 MB
Binary file not shown.

0 commit comments

Comments
 (0)