Skip to content

Commit 72ed863

Browse files
authored
Prevent lua from garbage collecting possibly-stale subframes (scp-fs2open#6866)
1 parent d6cb989 commit 72ed863

2 files changed

Lines changed: 16 additions & 7 deletions

File tree

code/scripting/api/objs/texture.cpp

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,13 @@
66
#define BMPMAN_INTERNAL
77
#include "bmpman/bm_internal.h"
88

9-
109
namespace scripting {
1110
namespace api {
1211

1312
texture_h::texture_h() = default;
14-
texture_h::texture_h(int bm, bool refcount) : handle(bm) {
13+
texture_h::texture_h(int bm, bool refcount, int parent_bm) : handle(bm), parent_handle(parent_bm) {
1514
if (refcount && isValid())
16-
bm_get_entry(bm)->load_count++;
15+
bm_get_entry(parent_bm != -1 ? parent_bm : bm)->load_count++;
1716
}
1817
texture_h::~texture_h()
1918
{
@@ -28,15 +27,22 @@ texture_h::~texture_h()
2827
//the textures using load_count. Anything that creates a texture object must also increase load count, unless it is
2928
//created in a way that already increases load_count (like bm_load). That way, a texture going out of scope needs to be
3029
//released and is safed against memleaks. -Lafiel
31-
bm_release(handle);
30+
//Note 2: Some textures, notably subframes of animations, aren't first-class bmpman citizens and mustn't be released directly.
31+
//Otherwise it is possible (and has been observed in practice) that the parent texture get's deleted before all dependent objects,
32+
//causing this release of the dependent object to clear unrelated textures that were assigned the previously freed spots.
33+
//So instead, both lock and later unlock the parent texture rather than this child texture. -Lafiel
34+
bm_release(parent_handle != -1 ? parent_handle : handle);
3235
}
3336
bool texture_h::isValid() const { return bm_is_valid(handle) != 0; }
37+
3438
texture_h::texture_h(texture_h&& other) noexcept {
3539
*this = std::move(other);
3640
}
3741
texture_h& texture_h::operator=(texture_h&& other) noexcept {
38-
if (this != &other)
42+
if (this != &other) {
3943
std::swap(handle, other.handle);
44+
std::swap(parent_handle, other.parent_handle);
45+
}
4046
return *this;
4147
}
4248

@@ -92,7 +98,7 @@ ADE_INDEXER(l_Texture, "number",
9298
//Get actual texture handle
9399
frame = first + frame;
94100

95-
return ade_set_args(L, "o", l_Texture.Set(texture_h(frame)));
101+
return ade_set_args(L, "o", l_Texture.Set(texture_h(frame, true, first)));
96102
}
97103

98104
ADE_FUNC(isValid, l_Texture, NULL, "Detects whether handle is valid", "boolean", "true if valid, false if handle is invalid, nil if a syntax/type error occurs")
@@ -139,6 +145,8 @@ ADE_FUNC(destroyRenderTarget, l_Texture, nullptr, "Destroys a texture's render t
139145

140146
bm_release_rendertarget(th->handle);
141147

148+
th->handle = -1;
149+
142150
return ADE_RETURN_NIL;
143151
}
144152

code/scripting/api/objs/texture.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,10 @@ namespace api {
99

1010
struct texture_h {
1111
int handle = -1;
12+
int parent_handle = -1;
1213

1314
texture_h();
14-
explicit texture_h(int bm, bool refcount = true);
15+
explicit texture_h(int bm, bool refcount = true, int parent_handle = -1);
1516

1617
~texture_h();
1718

0 commit comments

Comments
 (0)