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

@@ -364,7 +364,7 @@ private:
   _ARGSPECMEM
 };
 
-template <class R _COMMA _TMPLARG>
+template <class R _COMMA _TMPLARG, class F = gsi::return_by_value>
 class _NAME(StaticMethod)
   : public StaticMethodBase
 {
@@ -384,7 +384,11 @@ public:
   {
     this->clear ();
     _ADDARGS
-    this->template set_return<R> ();
+    if (tl::value_from_type (typename F::is_factory ())) {
+      this->template set_return_new<R> ();
+    } else {
+      this->template set_return<R> ();
+    }
   }
 
   virtual MethodBase *clone () const 
@@ -408,50 +412,6 @@ private:
   _ARGSPECMEM
 };
 
-template <class X _COMMA _TMPLARG>
-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<X *> ();
-  }
-
-  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<X *> ((*m_m) (_ARGVARLIST));
-  }
-
-private:
-  X *(*m_m) (_FUNCARGLIST);
-  _ARGSPECMEM
-};
-
 //  pointer iterator method descriptors
 
 template <class X, class R _COMMA _TMPLARG>
@@ -1352,6 +1312,22 @@ method (const std::string &name, R (X::*m) (_FUNCARGLIST) _COMMA _ARGSPECS, cons
 }
 #endif
 
+template <class X, class R _COMMA _TMPLARG>
+Methods
+factory (const std::string &name, R *(X::*m) (_FUNCARGLIST), const std::string &doc = std::string ())
+{
+  return Methods (new _NAME(Method) <X, R * _COMMA _FUNCARGLIST, gsi::return_new_object> (name, m, doc));
+}
+
+#if _COUNT != 0
+template <class X, class R _COMMA _TMPLARG _COMMA _TMPLARGSPECS>
+Methods
+factory (const std::string &name, R *(X::*m) (_FUNCARGLIST) _COMMA _ARGSPECS, const std::string &doc = std::string ())
+{
+  return Methods ((new _NAME(Method) <X, R * _COMMA _FUNCARGLIST, gsi::return_new_object> (name, m, doc))->add_args (_ARGSPECARGS));
+}
+#endif
+
 template <class X, class R _COMMA _TMPLARG>
 Methods
 method_ext (const std::string &name, R (*xm) (X * _COMMA _FUNCARGLIST), const std::string &doc = std::string ())
@@ -1388,7 +1364,7 @@ template <class X _COMMA _TMPLARG>
 Methods
 constructor (const std::string &name, X *(*m) (_FUNCARGLIST), const std::string &doc = std::string ())
 {
-  return Methods (new _NAME(Constructor) <X _COMMA _FUNCARGLIST> (name, m, doc));
+  return Methods (new _NAME(StaticMethod) <X * _COMMA _FUNCARGLIST, gsi::return_new_object> (name, m, doc));
 }
 
 #if _COUNT != 0
@@ -1396,7 +1372,7 @@ template <class X _COMMA _TMPLARG _COMMA _TMPLARGSPECS>
 Methods
 constructor (const std::string &name, X *(*m) (_FUNCARGLIST) _COMMA _ARGSPECS, const std::string &doc = std::string ())
 {
-  return Methods ((new _NAME(Constructor) <X _COMMA _FUNCARGLIST> (name, m, doc))->add_args (_ARGSPECARGS));
+  return Methods ((new _NAME(StaticMethod) <X * _COMMA _FUNCARGLIST, gsi::return_new_object> (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 <class R _COMMA _TMPLARG>
+Methods
+factory (const std::string &name, R *(*m) (_FUNCARGLIST), const std::string &doc = std::string ())
+{
+  return Methods (new _NAME(StaticMethod) <R * _COMMA _FUNCARGLIST, gsi::return_new_object> (name, m, doc));
+}
+
+#if _COUNT != 0
+template <class R _COMMA _TMPLARG _COMMA _TMPLARGSPECS>
+Methods
+factory (const std::string &name, R *(*m) (_FUNCARGLIST) _COMMA _ARGSPECS, const std::string &doc = std::string ())
+{
+  return Methods ((new _NAME(StaticMethod) <R * _COMMA _FUNCARGLIST, gsi::return_new_object> (name, m, doc))->add_args (_ARGSPECARGS));
+}
+#endif
+
 template <class X, class R _COMMA _TMPLARG>
 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 <class X, class R _COMMA _TMPLARG>
+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) <X, R _COMMA _FUNCARGLIST, gsi::return_new_object> (name, m, doc, cb));
+}
+
+#if _COUNT != 0
+template <class X, class R _COMMA _TMPLARG _COMMA _TMPLARGSPECS>
+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) <X, R _COMMA _FUNCARGLIST, gsi::return_new_object> (name, m, doc, cb))->add_args (_ARGSPECARGS));
+}
+#endif
+
 template <class X, class R _COMMA _TMPLARG>
 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 <class X, class R _COMMA _TMPLARG>
+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) <X, R _COMMA _FUNCARGLIST, gsi::return_new_object> (name, m, doc, cb));
+}
+
+#if _COUNT != 0
+template <class X, class R _COMMA _TMPLARG _COMMA _TMPLARGSPECS>
+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) <X, R _COMMA _FUNCARGLIST, gsi::return_new_object> (name, m, doc, cb))->add_args (_ARGSPECARGS));
+}
+#endif
+
 //  pointer iterators
 
 template <class R _COMMA _TMPLARG>

src/gsi/unit_tests/gsiTest.cc

@@ -1158,5 +1158,86 @@ gsi::Class<SE> 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, int> (&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, GObject *, int> (&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<GObject> decl_gobject_base ("GObjectBase",
+  gsi::method_ext ("g_virtual", &g_virtual) +
+  gsi::Methods()
+);
+
+static gsi::Class<GObject_P> 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<GFactory> decl_gfactory_base ("GFactoryBase",
+  gsi::factory ("create_f", &GFactory::create_f)
+);
+
+static gsi::Class<GFactory_P> decl_gfactory (decl_gfactory_base, "GFactory",
+  gsi::method_ext ("f", &f_org) +
+  gsi::factory_callback ("f", &GFactory_P::f, &GFactory_P::f_cb)
+);
+
 }
 

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
 {

src/laybasic/laybasic/gsiDeclLayPlugin.cc

@@ -509,7 +509,7 @@ Class<gsi::PluginFactoryBase> 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"

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

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()