From 9537876bba38b31a7ba6be6ab79f21b335c7b659 Mon Sep 17 00:00:00 2001 From: Thomas Geymayer Date: Sun, 23 Nov 2014 23:39:56 +0100 Subject: [PATCH] Nasal: add an naRef to ghosts to allow for proper gc of dependent objects/ghosts. This allows for binding the lifetime of any nasal object to the lifetime of a ghost. Otherwise circular references from objects saved within the ghost would prevent the ghost from being garbage collected. --- simgear/nasal/cppbind/CMakeLists.txt | 11 ++- simgear/nasal/cppbind/test/TestContext.hxx | 76 +++++++++++++++ .../nasal/cppbind/{ => test}/cppbind_test.cxx | 7 +- .../cppbind/{ => test}/cppbind_test_ghost.cxx | 4 +- simgear/nasal/cppbind/test/nasal_gc_test.cxx | 93 +++++++++++++++++++ .../cppbind/{ => test}/nasal_num_test.cxx | 48 +--------- simgear/nasal/data.h | 1 + simgear/nasal/gc.c | 7 +- simgear/nasal/misc.c | 18 +++- simgear/nasal/nasal.h | 10 ++ 10 files changed, 216 insertions(+), 59 deletions(-) create mode 100644 simgear/nasal/cppbind/test/TestContext.hxx rename simgear/nasal/cppbind/{ => test}/cppbind_test.cxx (99%) rename simgear/nasal/cppbind/{ => test}/cppbind_test_ghost.cxx (98%) create mode 100644 simgear/nasal/cppbind/test/nasal_gc_test.cxx rename simgear/nasal/cppbind/{ => test}/nasal_num_test.cxx (63%) diff --git a/simgear/nasal/cppbind/CMakeLists.txt b/simgear/nasal/cppbind/CMakeLists.txt index 1e52d9b9..9d005394 100644 --- a/simgear/nasal/cppbind/CMakeLists.txt +++ b/simgear/nasal/cppbind/CMakeLists.txt @@ -35,16 +35,21 @@ simgear_component(nasal/cppbind nasal/cppbind "${SOURCES}" "${HEADERS}") simgear_component(nasal/cppbind/detail nasal/cppbind/detail "" "${DETAIL_HEADERS}") add_boost_test(cppbind_ghost - SOURCES cppbind_test_ghost.cxx + SOURCES test/cppbind_test_ghost.cxx LIBRARIES ${TEST_LIBS} ) add_boost_test(cppbind_misc - SOURCES cppbind_test.cxx + SOURCES test/cppbind_test.cxx + LIBRARIES ${TEST_LIBS} +) + +add_boost_test(nasal_gc_test + SOURCES test/nasal_gc_test.cxx LIBRARIES ${TEST_LIBS} ) add_boost_test(nasal_num - SOURCES nasal_num_test.cxx + SOURCES test/nasal_num_test.cxx LIBRARIES ${TEST_LIBS} ) \ No newline at end of file diff --git a/simgear/nasal/cppbind/test/TestContext.hxx b/simgear/nasal/cppbind/test/TestContext.hxx new file mode 100644 index 00000000..69c2113e --- /dev/null +++ b/simgear/nasal/cppbind/test/TestContext.hxx @@ -0,0 +1,76 @@ +///@file +/// Nasal context for testing and executing code +/// +// Copyright (C) 2014 Thomas Geymayer +// +// This library is free software; you can redistribute it and/or +// modify it under the terms of the GNU Library General Public +// License as published by the Free Software Foundation; either +// version 2 of the License, or (at your option) any later version. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// Library General Public License for more details. +// +// You should have received a copy of the GNU Library General Public +// License along with this library; if not, write to the Free Software +// Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + +#ifndef SG_NASAL_TESTCONTEXT_HXX_ +#define SG_NASAL_TESTCONTEXT_HXX_ + +#include + +class TestContext: + public nasal::CallContext +{ + public: + TestContext(): + CallContext(naNewContext(), naNil(), 0, 0) + {} + + ~TestContext() + { + naFreeContext(c); + } + + void runGC() + { + naFreeContext(c); + naGC(); + c = naNewContext(); + } + + template + T from_str(const std::string& str) + { + return from_nasal(to_nasal(str)); + } + + naRef exec(const std::string& code_str, nasal::Me me) + { + int err_line = -1; + naRef code = naParseCode( c, to_nasal(""), 0, + (char*)code_str.c_str(), code_str.length(), + &err_line ); + if( !naIsCode(code) ) + throw std::runtime_error("Failed to parse code: " + code_str); + + return naCallMethod(code, me, 0, 0, naNil()); + } + + template + T exec(const std::string& code) + { + return from_nasal(exec(code, naNil())); + } + + template + T convert(const std::string& str) + { + return from_nasal(to_nasal(str)); + } +}; + +#endif /* SG_NASAL_TESTCONTEXT_HXX_ */ diff --git a/simgear/nasal/cppbind/cppbind_test.cxx b/simgear/nasal/cppbind/test/cppbind_test.cxx similarity index 99% rename from simgear/nasal/cppbind/cppbind_test.cxx rename to simgear/nasal/cppbind/test/cppbind_test.cxx index 2ea918d8..b4371f18 100644 --- a/simgear/nasal/cppbind/cppbind_test.cxx +++ b/simgear/nasal/cppbind/test/cppbind_test.cxx @@ -1,10 +1,9 @@ #define BOOST_TEST_MODULE cppbind #include -#include "Ghost.hxx" -#include "NasalHash.hxx" -#include "NasalString.hxx" - +#include +#include +#include #include #include diff --git a/simgear/nasal/cppbind/cppbind_test_ghost.cxx b/simgear/nasal/cppbind/test/cppbind_test_ghost.cxx similarity index 98% rename from simgear/nasal/cppbind/cppbind_test_ghost.cxx rename to simgear/nasal/cppbind/test/cppbind_test_ghost.cxx index 98924989..d74f6a14 100644 --- a/simgear/nasal/cppbind/cppbind_test_ghost.cxx +++ b/simgear/nasal/cppbind/test/cppbind_test_ghost.cxx @@ -1,8 +1,8 @@ #define BOOST_TEST_MODULE cppbind #include -#include "Ghost.hxx" -#include "NasalContext.hxx" +#include +#include #include #include diff --git a/simgear/nasal/cppbind/test/nasal_gc_test.cxx b/simgear/nasal/cppbind/test/nasal_gc_test.cxx new file mode 100644 index 00000000..c1cb6ca3 --- /dev/null +++ b/simgear/nasal/cppbind/test/nasal_gc_test.cxx @@ -0,0 +1,93 @@ +#define BOOST_TEST_MODULE nasal +#include + +#include "TestContext.hxx" +#include +#include + +static std::set active_instances; + +static void ghost_destroy(void* p) +{ + active_instances.erase((intptr_t)p); +} + +static naGhostType ghost_type = { + &ghost_destroy, + "TestGhost", + 0, // get_member + 0 // set_member +}; + +static naRef createTestGhost(TestContext& c, intptr_t p) +{ + active_instances.insert(p); + return naNewGhost(c.c, &ghost_type, (void*)p); +} + +//------------------------------------------------------------------------------ +BOOST_AUTO_TEST_CASE( ghost_gc ) +{ + TestContext c; + BOOST_REQUIRE(active_instances.empty()); + + //----------------------------------------------- + // Just create ghosts and let the gc destroy them + + naRef g1 = createTestGhost(c, 1), + g2 = createTestGhost(c, 2); + + BOOST_CHECK_EQUAL(active_instances.count(1), 1); + BOOST_CHECK_EQUAL(active_instances.count(2), 1); + BOOST_CHECK_EQUAL(active_instances.size(), 2); + + c.runGC(); + + BOOST_REQUIRE(active_instances.empty()); + + + //----------------------------------------------- + // Create some more ghosts and save one instance + // from being garbage collected. + + g1 = createTestGhost(c, 1); + g2 = createTestGhost(c, 2); + + int gc1 = naGCSave(g1); + c.runGC(); + + BOOST_CHECK_EQUAL(active_instances.count(1), 1); + BOOST_CHECK_EQUAL(active_instances.size(), 1); + + naGCRelease(gc1); + c.runGC(); + + BOOST_REQUIRE(active_instances.empty()); + + + //----------------------------------------------- + // Now test attaching one ghost to another + + g1 = createTestGhost(c, 1); + g2 = createTestGhost(c, 2); + + gc1 = naGCSave(g1); + naGhost_setData(g1, g2); // bind lifetime of g2 to g1... + + c.runGC(); + + BOOST_CHECK_EQUAL(active_instances.count(1), 1); + BOOST_CHECK_EQUAL(active_instances.count(2), 1); + BOOST_CHECK_EQUAL(active_instances.size(), 2); + + naGhost_setData(g1, naNil()); // cut connection + c.runGC(); + + BOOST_CHECK_EQUAL(active_instances.count(1), 1); + BOOST_CHECK_EQUAL(active_instances.size(), 1); + + naGCRelease(gc1); + c.runGC(); + + BOOST_REQUIRE(active_instances.empty()); +} diff --git a/simgear/nasal/cppbind/nasal_num_test.cxx b/simgear/nasal/cppbind/test/nasal_num_test.cxx similarity index 63% rename from simgear/nasal/cppbind/nasal_num_test.cxx rename to simgear/nasal/cppbind/test/nasal_num_test.cxx index e1e7dd0f..99000662 100644 --- a/simgear/nasal/cppbind/nasal_num_test.cxx +++ b/simgear/nasal/cppbind/test/nasal_num_test.cxx @@ -1,51 +1,7 @@ -#define BOOST_TEST_MODULE cppbind +#define BOOST_TEST_MODULE nasal #include -#include "NasalCallContext.hxx" - -class TestContext: - public nasal::CallContext -{ - public: - TestContext(): - CallContext(naNewContext(), naNil(), 0, 0) - {} - - ~TestContext() - { - naFreeContext(c); - } - - template - T from_str(const std::string& str) - { - return from_nasal(to_nasal(str)); - } - - naRef exec(const std::string& code_str, nasal::Me me) - { - int err_line = -1; - naRef code = naParseCode( c, to_nasal(""), 0, - (char*)code_str.c_str(), code_str.length(), - &err_line ); - if( !naIsCode(code) ) - throw std::runtime_error("Failed to parse code: " + code_str); - - return naCallMethod(code, me, 0, 0, naNil()); - } - - template - T exec(const std::string& code) - { - return from_nasal(exec(code, naNil())); - } - - template - T convert(const std::string& str) - { - return from_nasal(to_nasal(str)); - } -}; +#include "TestContext.hxx" static void runNumTests( double (TestContext::*test_double)(const std::string&), int (TestContext::*test_int)(const std::string&) ) diff --git a/simgear/nasal/data.h b/simgear/nasal/data.h index 25f7008e..9a91c3e8 100644 --- a/simgear/nasal/data.h +++ b/simgear/nasal/data.h @@ -175,6 +175,7 @@ struct naGhost { GC_HEADER; naGhostType* gtype; void* ptr; + naRef data; //!< Nasal data bound to the lifetime of the ghost. }; struct naPool { diff --git a/simgear/nasal/gc.c b/simgear/nasal/gc.c index a965aa79..5ac9c43c 100644 --- a/simgear/nasal/gc.c +++ b/simgear/nasal/gc.c @@ -177,7 +177,7 @@ static void newBlock(struct naPool* p, int need) newb->next = p->blocks; p->blocks = newb; naBZero(newb->block, need * p->elemsz); - + if(need > p->freesz - p->freetop) need = p->freesz - p->freetop; p->nfree = 0; p->free = p->free0 + p->freetop; @@ -263,6 +263,9 @@ static void mark(naRef r) mark(PTR(r).func->namespace); mark(PTR(r).func->next); break; + case T_GHOST: + mark(PTR(r).ghost->data); + break; } } @@ -300,7 +303,7 @@ static void reap(struct naPool* p) // allocs of this type until the next collection globals->allocCount += total/2; - + // Allocate more if necessary (try to keep 25-50% of the objects // available) if(p->nfree < total/4) { diff --git a/simgear/nasal/misc.c b/simgear/nasal/misc.c index 5a531d53..0a5c8615 100644 --- a/simgear/nasal/misc.c +++ b/simgear/nasal/misc.c @@ -143,10 +143,11 @@ naRef naNewGhost(naContext c, naGhostType* type, void* ptr) // ensure 'simple' ghost users don't see garbage for these fields type->get_member = 0; type->set_member = 0; - + ghost = naNew(c, T_GHOST); PTR(ghost).ghost->gtype = type; PTR(ghost).ghost->ptr = ptr; + PTR(ghost).ghost->data = naNil(); return ghost; } @@ -155,6 +156,7 @@ naRef naNewGhost2(naContext c, naGhostType* t, void* ptr) naRef ghost = naNew(c, T_GHOST); PTR(ghost).ghost->gtype = t; PTR(ghost).ghost->ptr = ptr; + PTR(ghost).ghost->data = naNil(); return ghost; } @@ -170,9 +172,21 @@ void* naGhost_ptr(naRef ghost) return PTR(ghost).ghost->ptr; } +void naGhost_setData(naRef ghost, naRef data) +{ + if(IS_GHOST(ghost)) + PTR(ghost).ghost->data = data; +} + +naRef naGhost_data(naRef ghost) +{ + if(!IS_GHOST(ghost)) return naNil(); + return PTR(ghost).ghost->data; +} + naRef naNil() { - naRef r; + naRef r; SETPTR(r, 0); return r; } diff --git a/simgear/nasal/nasal.h b/simgear/nasal/nasal.h index 83272447..8b6b2c92 100644 --- a/simgear/nasal/nasal.h +++ b/simgear/nasal/nasal.h @@ -281,6 +281,16 @@ naRef naNewGhost(naContext c, naGhostType* t, void* ghost); naRef naNewGhost2(naContext c, naGhostType* t, void* ghost); naGhostType* naGhost_type(naRef ghost); void* naGhost_ptr(naRef ghost); +/** + * Attach a nasal object to the given ghost. Binds the lifetime of @a data to + * the lifetime of the @a ghost. + */ +void naGhost_setData(naRef ghost, naRef data); +/** + * Retrieve the object attached to the @a ghost, previously set with + * naGhost_setData(). + */ +naRef naGhost_data(naRef ghost); int naIsGhost(naRef r); // Acquires a "modification lock" on a context, allowing the C code to -- 2.39.5