From 1cea7dfd23063ab3aa73bc485c2532f7d221ec0e Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Mon, 11 Dec 2017 23:51:00 +0100 Subject: [PATCH] Fixed #33 (Plugin factory not working when using with Python) The fix consisted of introducing "factory" type virtual methods which ensure that a reference is held to the returned object. This is important for implementing factory methods in Python. Without this, the object get destroyed before we have a chance to increment the reference count. --- src/gsi/gsi/gsiMethodsVar.h | 120 +++++++++++++--------- src/gsi/unit_tests/gsiTest.cc | 81 +++++++++++++++ src/gsi/unit_tests/gsiTest.h | 48 +++++++++ src/laybasic/laybasic/gsiDeclLayPlugin.cc | 2 +- src/pya/pya/pyaObject.cc | 2 +- testdata/python/basic.py | 27 +++++ 6 files changed, 230 insertions(+), 50 deletions(-) diff --git a/src/gsi/gsi/gsiMethodsVar.h b/src/gsi/gsi/gsiMethodsVar.h index 4adedbc88..b7b5c58dc 100644 --- a/src/gsi/gsi/gsiMethodsVar.h +++ b/src/gsi/gsi/gsiMethodsVar.h @@ -364,7 +364,7 @@ private: _ARGSPECMEM }; -template +template class _NAME(StaticMethod) : public StaticMethodBase { @@ -384,7 +384,11 @@ public: { this->clear (); _ADDARGS - this->template set_return (); + if (tl::value_from_type (typename F::is_factory ())) { + this->template set_return_new (); + } else { + this->template set_return (); + } } virtual MethodBase *clone () const @@ -408,50 +412,6 @@ private: _ARGSPECMEM }; -template -class _NAME(Constructor) - : public StaticMethodBase -{ -public: - _NAME(Constructor) (const std::string &name, X *(*m) (_FUNCARGLIST), const std::string &doc) - : StaticMethodBase (name, doc), m_m (m) - { - } - - _NAME(Constructor) *add_args (_ARGSPEC) - { - _ARGSPECINIT; - return this; - } - - void initialize () - { - this->clear (); - _ADDARGS - this->template set_return_new (); - } - - virtual MethodBase *clone () const - { - return new _NAME(Constructor) (*this); - } - -#if _COUNT != 0 - virtual void call (void *, SerialArgs &args, SerialArgs &ret) const -#else - virtual void call (void *, SerialArgs &, SerialArgs &ret) const -#endif - { - this->mark_called (); - _GETARGVARS; - ret.write ((*m_m) (_ARGVARLIST)); - } - -private: - X *(*m_m) (_FUNCARGLIST); - _ARGSPECMEM -}; - // pointer iterator method descriptors template @@ -1352,6 +1312,22 @@ method (const std::string &name, R (X::*m) (_FUNCARGLIST) _COMMA _ARGSPECS, cons } #endif +template +Methods +factory (const std::string &name, R *(X::*m) (_FUNCARGLIST), const std::string &doc = std::string ()) +{ + return Methods (new _NAME(Method) (name, m, doc)); +} + +#if _COUNT != 0 +template +Methods +factory (const std::string &name, R *(X::*m) (_FUNCARGLIST) _COMMA _ARGSPECS, const std::string &doc = std::string ()) +{ + return Methods ((new _NAME(Method) (name, m, doc))->add_args (_ARGSPECARGS)); +} +#endif + template Methods method_ext (const std::string &name, R (*xm) (X * _COMMA _FUNCARGLIST), const std::string &doc = std::string ()) @@ -1388,7 +1364,7 @@ template Methods constructor (const std::string &name, X *(*m) (_FUNCARGLIST), const std::string &doc = std::string ()) { - return Methods (new _NAME(Constructor) (name, m, doc)); + return Methods (new _NAME(StaticMethod) (name, m, doc)); } #if _COUNT != 0 @@ -1396,7 +1372,7 @@ template Methods constructor (const std::string &name, X *(*m) (_FUNCARGLIST) _COMMA _ARGSPECS, const std::string &doc = std::string ()) { - return Methods ((new _NAME(Constructor) (name, m, doc))->add_args (_ARGSPECARGS)); + return Methods ((new _NAME(StaticMethod) (name, m, doc))->add_args (_ARGSPECARGS)); } #endif @@ -1416,6 +1392,22 @@ method (const std::string &name, R (*m) (_FUNCARGLIST) _COMMA _ARGSPECS, const s } #endif +template +Methods +factory (const std::string &name, R *(*m) (_FUNCARGLIST), const std::string &doc = std::string ()) +{ + return Methods (new _NAME(StaticMethod) (name, m, doc)); +} + +#if _COUNT != 0 +template +Methods +factory (const std::string &name, R *(*m) (_FUNCARGLIST) _COMMA _ARGSPECS, const std::string &doc = std::string ()) +{ + return Methods ((new _NAME(StaticMethod) (name, m, doc))->add_args (_ARGSPECARGS)); +} +#endif + template Methods callback (const std::string &name, R (X::*m) (_FUNCARGLIST), Callback X::*cb, const std::string &doc = std::string ()) @@ -1432,6 +1424,22 @@ callback (const std::string &name, R (X::*m) (_FUNCARGLIST), Callback X::*cb _CO } #endif +template +Methods +factory_callback (const std::string &name, R (X::*m) (_FUNCARGLIST), Callback X::*cb, const std::string &doc = std::string ()) +{ + return Methods (new _NAME(Method) (name, m, doc, cb)); +} + +#if _COUNT != 0 +template +Methods +factory_callback (const std::string &name, R (X::*m) (_FUNCARGLIST), Callback X::*cb _COMMA _ARGSPECS, const std::string &doc = std::string ()) +{ + return Methods ((new _NAME(Method) (name, m, doc, cb))->add_args (_ARGSPECARGS)); +} +#endif + template Methods method (const std::string &name, R (X::*m) (_FUNCARGLIST) const, const std::string &doc = std::string ()) @@ -1480,6 +1488,22 @@ callback (const std::string &name, R (X::*m) (_FUNCARGLIST) const, Callback X::* } #endif +template +Methods +factory_callback (const std::string &name, R (X::*m) (_FUNCARGLIST) const, Callback X::*cb, const std::string &doc = std::string ()) +{ + return Methods (new _NAME(ConstMethod) (name, m, doc, cb)); +} + +#if _COUNT != 0 +template +Methods +factory_callback (const std::string &name, R (X::*m) (_FUNCARGLIST) const, Callback X::*cb _COMMA _ARGSPECS, const std::string &doc = std::string ()) +{ + return Methods ((new _NAME(ConstMethod) (name, m, doc, cb))->add_args (_ARGSPECARGS)); +} +#endif + // pointer iterators template diff --git a/src/gsi/unit_tests/gsiTest.cc b/src/gsi/unit_tests/gsiTest.cc index a9b0f8e38..9a9b1394e 100644 --- a/src/gsi/unit_tests/gsiTest.cc +++ b/src/gsi/unit_tests/gsiTest.cc @@ -1158,5 +1158,86 @@ gsi::Class decl_se ("SE", gsi::event ("s2", &SE::s2) ); +// ------------------------------------------------------------------ +// G and GFactory implementation and GSI declarations + +GObject::GObject () +{ + ++s_g_inst_count; +} + +GObject::~GObject () +{ + --s_g_inst_count; +} + +size_t GObject::s_g_inst_count = 0; + +GObject_P::GObject_P () + : GObject () +{ + // .. nothing yet .. +} + +int GObject_P::g () +{ + return g_cb.can_issue () ? g_cb.issue (&GObject::g) : GObject::g (); +} + +GFactory::GFactory () +{ + // .. nothing yet .. +} + +GFactory::~GFactory () +{ + // .. nothing yet .. +} + +GFactory_P::GFactory_P () +{ + // .. nothing yet .. +} + +GObject *GFactory_P::f (int z) +{ + return f_cb.can_issue () ? f_cb.issue (&GFactory::f, z) : GFactory::f (z); +} + +int g_org (GObject_P *go) +{ + return go->GObject::g (); +} + +int g_virtual (GObject *go) +{ + return go->g (); +} + +static gsi::Class decl_gobject_base ("GObjectBase", + gsi::method_ext ("g_virtual", &g_virtual) + + gsi::Methods() +); + +static gsi::Class decl_gobject (decl_gobject_base, "GObject", + gsi::method_ext ("g_org", &g_org) + + gsi::callback ("g", &GObject_P::g, &GObject_P::g_cb) + + gsi::method ("g_inst_count", &GObject::g_inst_count) +); + +GObject *f_org (GFactory_P *fo, int z) +{ + return fo->GFactory::f (z); +} + +static gsi::Class decl_gfactory_base ("GFactoryBase", + gsi::factory ("create_f", &GFactory::create_f) +); + +static gsi::Class decl_gfactory (decl_gfactory_base, "GFactory", + gsi::method_ext ("f", &f_org) + + gsi::factory_callback ("f", &GFactory_P::f, &GFactory_P::f_cb) +); + } diff --git a/src/gsi/unit_tests/gsiTest.h b/src/gsi/unit_tests/gsiTest.h index bffa6a2b1..e8b31e5c8 100644 --- a/src/gsi/unit_tests/gsiTest.h +++ b/src/gsi/unit_tests/gsiTest.h @@ -1044,6 +1044,54 @@ public: gsi::Callback f_cb; }; +// An object that is produced by a factory +class GObject +{ +public: + GObject (); + virtual ~GObject (); + virtual int g () { return 0; } + static size_t g_inst_count () + { + return s_g_inst_count; + } + +private: + static size_t s_g_inst_count; +}; + +class GObject_P : public GObject +{ +public: + GObject_P (); + virtual int g (); + + gsi::Callback g_cb; +}; + +// This is the factory for G +class GFactory +{ +public: + GFactory (); + virtual ~GFactory (); + + virtual GObject *f (int /*z*/) { return 0; } + static GObject *create_f (GFactory *g_factory, int z) + { + return g_factory->f (z); + } +}; + +class GFactory_P : public GFactory +{ +public: + GFactory_P (); + virtual GObject *f (int z); + + gsi::Callback f_cb; +}; + class SQ : public QObject { diff --git a/src/laybasic/laybasic/gsiDeclLayPlugin.cc b/src/laybasic/laybasic/gsiDeclLayPlugin.cc index 77fbacbe2..bd3e17e85 100644 --- a/src/laybasic/laybasic/gsiDeclLayPlugin.cc +++ b/src/laybasic/laybasic/gsiDeclLayPlugin.cc @@ -509,7 +509,7 @@ Class decl_PluginFactory ("PluginFactory", "@args root\n" "@param root The reference to the \\MainWindow object\n" ) + - callback ("create_plugin", &gsi::PluginFactoryBase::create_plugin_gsi, &gsi::PluginFactoryBase::f_create_plugin, + factory_callback ("create_plugin", &gsi::PluginFactoryBase::create_plugin_gsi, &gsi::PluginFactoryBase::f_create_plugin, "@brief Creates the plugin\n" "This is the basic functionality that the factory must provide. This method must create a plugin of the " "specific type.\n" diff --git a/src/pya/pya/pyaObject.cc b/src/pya/pya/pyaObject.cc index 7a1d95fdb..c9204565a 100644 --- a/src/pya/pya/pyaObject.cc +++ b/src/pya/pya/pyaObject.cc @@ -173,7 +173,7 @@ Callee::call (int id, gsi::SerialArgs &args, gsi::SerialArgs &ret) const } tl::Heap heap; - push_arg (meth->ret_type (), ret, result.get (), heap); + push_arg (meth->ret_type (), ret, meth->ret_type().pass_obj() ? result.release() : result.get (), heap); // a Python callback must not leave temporary objects diff --git a/testdata/python/basic.py b/testdata/python/basic.py index 38595d8e8..fec8d53c1 100644 --- a/testdata/python/basic.py +++ b/testdata/python/basic.py @@ -92,6 +92,22 @@ def ph(x): else: return x.replace("X", "") +class PyGObject(pya.GObject): + z = -1 + def __init__(self, z): + super(PyGObject, self).__init__() + self.z = z + # reimplementation of "virtual int g()" + def g(self): + return self.z*2 + +class PyGFactory(pya.GFactory): + def __init__(self): + super(PyGFactory, self).__init__() + # reimplementation of "virtual GObject *f(int)" + def f(self, z): + return PyGObject(z) + class BasicTest(unittest.TestCase): def test_00(self): @@ -2693,6 +2709,17 @@ class BasicTest(unittest.TestCase): self.assertEqual(sc.got_s0a, 0) self.assertEqual(sc.got_s0b, 0) + # Custom factory implemented in Python + def test_80(self): + gc = pya.GObject.g_inst_count() + gf = PyGFactory() + go = pya.GFactory.create_f(gf, 17) + self.assertEqual(go.g_virtual(), 34) + self.assertEqual(go.g_org(), 0) + self.assertEqual(pya.GObject.g_inst_count(), gc + 1) + go = None + self.assertEqual(pya.GObject.g_inst_count(), gc) + # run unit tests if __name__ == '__main__': suite = unittest.TestSuite()