From 9758e660c4422dc8ca52c09b56a733d9652e6a82 Mon Sep 17 00:00:00 2001 From: Jonas Kulla Date: Fri, 26 Sep 2014 18:21:50 +0200 Subject: [PATCH] Tilemap/VX: Ensure proxy objects don't outlive their parents Either of these would previously crash (same for VX): tm = Tilemap.new at = tm.autotiles tm = nil GC.start at[0] = Bitmap.new(1, 1) tm = Tilemap.new at = tm.autotiles tm.dispose at[0] = Bitmap.new(1, 1) Funnily, this makes RMXP itself crash too, but crashing is never acceptable except for possibly resource exhaustion. --- binding-mri/tilemap-binding.cpp | 4 ++++ binding-mri/tilemapvx-binding.cpp | 4 ++++ binding-mruby/binding-util.cpp | 1 + binding-mruby/binding-util.h | 1 + binding-mruby/tilemap-binding.cpp | 9 +++++++-- src/tilemap.cpp | 14 ++++++++++---- src/tilemap.h | 3 ++- src/tilemapvx.cpp | 14 ++++++++++---- src/tilemapvx.h | 3 ++- 9 files changed, 41 insertions(+), 12 deletions(-) diff --git a/binding-mri/tilemap-binding.cpp b/binding-mri/tilemap-binding.cpp index bdb429f..2217012 100644 --- a/binding-mri/tilemap-binding.cpp +++ b/binding-mri/tilemap-binding.cpp @@ -94,6 +94,10 @@ RB_METHOD(tilemapInitialize) rb_iv_set(autotilesObj, "array", ary); + /* Circular reference so both objects are always + * alive at the same time */ + rb_iv_set(autotilesObj, "tilemap", self); + return self; } diff --git a/binding-mri/tilemapvx-binding.cpp b/binding-mri/tilemapvx-binding.cpp index 58a586d..c451e52 100644 --- a/binding-mri/tilemapvx-binding.cpp +++ b/binding-mri/tilemapvx-binding.cpp @@ -64,6 +64,10 @@ RB_METHOD(tilemapVXInitialize) rb_iv_set(autotilesObj, "array", ary); + /* Circular reference so both objects are always + * alive at the same time */ + rb_iv_set(autotilesObj, "tilemap", self); + return self; } diff --git a/binding-mruby/binding-util.cpp b/binding-mruby/binding-util.cpp index 8973b75..49c2563 100644 --- a/binding-mruby/binding-util.cpp +++ b/binding-mruby/binding-util.cpp @@ -41,6 +41,7 @@ struct SYMD(tone), SYMD(rect), SYMD(src_rect), + SYMD(tilemap), SYMD(tileset), SYMD(autotiles), SYMD(map_data), diff --git a/binding-mruby/binding-util.h b/binding-mruby/binding-util.h index 28e1211..5883e43 100644 --- a/binding-mruby/binding-util.h +++ b/binding-mruby/binding-util.h @@ -42,6 +42,7 @@ enum CommonSymbol CStone, CSrect, CSsrc_rect, + CStilemap, CStileset, CSautotiles, CSmap_data, diff --git a/binding-mruby/tilemap-binding.cpp b/binding-mruby/tilemap-binding.cpp index 1d4efcf..e121ba8 100644 --- a/binding-mruby/tilemap-binding.cpp +++ b/binding-mruby/tilemap-binding.cpp @@ -93,13 +93,18 @@ MRB_METHOD(tilemapInitialize) wrapProperty(mrb, self, &t->getAutotiles(), CSautotiles, TilemapAutotilesType); - mrb_value autotilesObj = mrb_iv_get(mrb, self, getMrbData(mrb)->symbols[CSautotiles]); + MrbData &mrbData = *getMrbData(mrb); + mrb_value autotilesObj = mrb_iv_get(mrb, self, mrbData.symbols[CSautotiles]); mrb_value ary = mrb_ary_new_capa(mrb, 7); for (int i = 0; i < 7; ++i) mrb_ary_push(mrb, ary, mrb_nil_value()); - mrb_iv_set(mrb, autotilesObj, getMrbData(mrb)->symbols[CSarray], ary); + mrb_iv_set(mrb, autotilesObj, mrbData.symbols[CSarray], ary); + + /* Circular reference so both objects are always + * alive at the same time */ + mrb_iv_set(mrb, autotilesObj, mrbData.symbols[CStilemap], self); return self; } diff --git a/src/tilemap.cpp b/src/tilemap.cpp index b92cc34..9878275 100644 --- a/src/tilemap.cpp +++ b/src/tilemap.cpp @@ -241,7 +241,6 @@ struct TilemapPrivate { Viewport *viewport; - Tilemap::Autotiles autotilesProxy; Bitmap *autotiles[autotileCount]; Bitmap *tileset; @@ -1183,6 +1182,9 @@ void ZLayer::finiUpdateZ(ZLayer *prev) void Tilemap::Autotiles::set(int i, Bitmap *bitmap) { + if (!p) + return; + if (i < 0 || i > autotileCount-1) return; @@ -1206,6 +1208,9 @@ void Tilemap::Autotiles::set(int i, Bitmap *bitmap) Bitmap *Tilemap::Autotiles::get(int i) const { + if (!p) + return 0; + if (i < 0 || i > autotileCount-1) return 0; @@ -1215,7 +1220,7 @@ Bitmap *Tilemap::Autotiles::get(int i) const Tilemap::Tilemap(Viewport *viewport) { p = new TilemapPrivate(viewport); - p->autotilesProxy.p = p; + atProxy.p = p; } Tilemap::~Tilemap() @@ -1244,11 +1249,11 @@ void Tilemap::update() p->tiles.aniIdx = 0; } -Tilemap::Autotiles &Tilemap::getAutotiles() const +Tilemap::Autotiles &Tilemap::getAutotiles() { guardDisposed(); - return p->autotilesProxy; + return atProxy; } DEF_ATTR_RD_SIMPLE(Tilemap, Viewport, Viewport*, p->viewport) @@ -1379,4 +1384,5 @@ void Tilemap::setOY(int value) void Tilemap::releaseResources() { delete p; + atProxy.p = 0; } diff --git a/src/tilemap.h b/src/tilemap.h index 6e68d92..9afe00f 100644 --- a/src/tilemap.h +++ b/src/tilemap.h @@ -55,7 +55,7 @@ public: void update(); - Autotiles &getAutotiles() const; + Autotiles &getAutotiles(); Viewport *getViewport() const; DECL_ATTR( Tileset, Bitmap* ) @@ -68,6 +68,7 @@ public: private: TilemapPrivate *p; + Autotiles atProxy; void releaseResources(); const char *klassName() const { return "tilemap"; } diff --git a/src/tilemapvx.cpp b/src/tilemapvx.cpp index fb5aae5..bbd925d 100644 --- a/src/tilemapvx.cpp +++ b/src/tilemapvx.cpp @@ -41,7 +41,6 @@ struct TilemapVXPrivate : public ViewportElement, TileAtlasVX::Reader { - TilemapVX::BitmapArray bitmapsProxy; Bitmap *bitmaps[BM_COUNT]; Table *mapData; @@ -358,6 +357,9 @@ struct TilemapVXPrivate : public ViewportElement, TileAtlasVX::Reader void TilemapVX::BitmapArray::set(int i, Bitmap *bitmap) { + if (!p) + return; + if (i < 0 || i >= BM_COUNT) return; @@ -378,6 +380,9 @@ void TilemapVX::BitmapArray::set(int i, Bitmap *bitmap) Bitmap *TilemapVX::BitmapArray::get(int i) const { + if (!p) + return 0; + if (i < 0 || i >= BM_COUNT) return 0; @@ -387,7 +392,7 @@ Bitmap *TilemapVX::BitmapArray::get(int i) const TilemapVX::TilemapVX(Viewport *viewport) { p = new TilemapVXPrivate(viewport); - p->bitmapsProxy.p = p; + bmProxy.p = p; } TilemapVX::~TilemapVX() @@ -413,11 +418,11 @@ void TilemapVX::update() p->aniOffset = Vec2(aniIdxA * 2 * 32, aniIdxC * 32); } -TilemapVX::BitmapArray &TilemapVX::getBitmapArray() const +TilemapVX::BitmapArray &TilemapVX::getBitmapArray() { guardDisposed(); - return p->bitmapsProxy; + return bmProxy; } DEF_ATTR_RD_SIMPLE(TilemapVX, MapData, Table*, p->mapData) @@ -521,4 +526,5 @@ void TilemapVX::setOY(int value) void TilemapVX::releaseResources() { delete p; + bmProxy.p = 0; } diff --git a/src/tilemapvx.h b/src/tilemapvx.h index 0e2b4be..a4df41d 100644 --- a/src/tilemapvx.h +++ b/src/tilemapvx.h @@ -54,7 +54,7 @@ public: void update(); - BitmapArray &getBitmapArray() const; + BitmapArray &getBitmapArray(); DECL_ATTR( Viewport, Viewport* ) DECL_ATTR( MapData, Table* ) @@ -66,6 +66,7 @@ public: private: TilemapVXPrivate *p; + BitmapArray bmProxy; void releaseResources(); const char *klassName() const { return "tilemap"; }