Issue 1029 - Crash on QTreeViewItem#setBackgroundColor (#1032)

* Fixed issue #1029

The problem happened because in the described case the
tl::Variant used as a intermediate container holds the
Python QBrush object and when it gets deleted, the
QBrush object is deleted too.

* Added tests

src/pya/pya/pyaMarshal.cc

@@ -114,6 +114,7 @@ public:
 
   virtual tl::Variant var () const;
   virtual void set (const tl::Variant &v);
+  const PythonPtr &ptr () const { return m_var; }
 
 private:
   PythonPtr m_var;
@@ -641,7 +642,7 @@ struct reader<gsi::ByteArrayType>
 };
 
 static
-PyObject *object_from_variant (tl::Variant &var, PYAObjectBase *self, const gsi::ArgType &atype)
+PyObject *object_from_variant (tl::Variant &var, PYAObjectBase *self, const gsi::ArgType &atype, bool transfer = false)
 {
   if (var.is_user()) {
 
@@ -657,7 +658,7 @@ PyObject *object_from_variant (tl::Variant &var, PYAObjectBase *self, const gsi:
     void *obj = var.to_user ();
     const gsi::ClassBase *cls = var.user_cls ()->gsi_cls ();
 
-    if (pass_obj) {
+    if (pass_obj || transfer) {
 
       if (holder) {
 
@@ -711,12 +712,17 @@ struct reader<gsi::VariantType>
       *ret = PythonRef (Py_None, false /*borrowed*/);
     } else {
       gsi::VariantAdaptorImpl<tl::Variant> *aa = dynamic_cast<gsi::VariantAdaptorImpl<tl::Variant> *> (a.get ());
+      PythonBasedVariantAdaptor *pa = dynamic_cast<PythonBasedVariantAdaptor *> (a.get ());
       if (aa) {
         //  A small optimization that saves one variant copy
         *ret = object_from_variant (aa->var_ref_nc (), self, atype);
+      } else if (pa) {
+        //  Optimization for Python to Python transfer
+        *ret = pa->ptr ();
       } else {
         tl::Variant v = a->var ();
-        *ret = object_from_variant (v, self, atype);
+        //  NOTE: as v may hold the object, we need to transfer ownership
+        *ret = object_from_variant (v, self, atype, true);
       }
     }
   }

src/rba/rba/rbaMarshal.cc

@@ -125,6 +125,7 @@ public:
 
   virtual tl::Variant var () const;
   virtual void set (const tl::Variant &v);
+  VALUE value () const { return m_var; }
 
 private:
   VALUE m_var;
@@ -662,7 +663,7 @@ struct reader<gsi::ByteArrayType>
   }
 };
 
-static VALUE object_from_variant (tl::Variant &var, Proxy *self, const gsi::ArgType &atype)
+static VALUE object_from_variant (tl::Variant &var, Proxy *self, const gsi::ArgType &atype, bool transfer = false)
 {
   if (var.is_user()) {
 
@@ -678,7 +679,7 @@ static VALUE object_from_variant (tl::Variant &var, Proxy *self, const gsi::ArgT
     void *obj = var.to_user ();
     const gsi::ClassBase *cls = var.user_cls ()->gsi_cls ();
 
-    if (pass_obj) {
+    if (pass_obj || transfer) {
 
       if (holder) {
 
@@ -732,12 +733,17 @@ struct reader<gsi::VariantType>
       *ret = Qnil;
     } else {
       gsi::VariantAdaptorImpl<tl::Variant> *aa = dynamic_cast<gsi::VariantAdaptorImpl<tl::Variant> *> (a.get ());
+      RubyBasedVariantAdaptor *pa = dynamic_cast<RubyBasedVariantAdaptor *> (a.get ());
       if (aa) {
         //  A small optimization that saves one variant copy
         *ret = object_from_variant (aa->var_ref_nc (), self, atype);
+      } else if (pa) {
+        //  Optimization for Ruby to Ruby transfer
+        *ret = pa->value ();
       } else {
         tl::Variant v = a->var ();
-        *ret = object_from_variant (v, self, atype);
+        //  NOTE: as v may hold the object, we need to transfer ownership
+        *ret = object_from_variant (v, self, atype, true);
       }
     }
   }

testdata/python/qtbinding.py

@@ -640,6 +640,15 @@ class QtBindingTest(unittest.TestCase):
     else:
       self.assertEqual(str(jsonData), 'b\'{"test":"test"}\'')
 
+  def test_54(self):
+
+    # issue #1029 (Crash for QBrush passed to setData)
+    item = pya.QTreeWidgetItem()
+    item.setBackground(0, pya.QBrush(pya.QColor(0xFF, 0xFF, 0x00)))
+    self.assertEqual(item.background(0).color.red, 255)
+    self.assertEqual(item.background(0).color.green, 255)
+    self.assertEqual(item.background(0).color.blue, 0)
+
 # run unit tests
 if __name__ == '__main__':
   suite = unittest.TestLoader().loadTestsFromTestCase(QtBindingTest)

testdata/ruby/qtbinding.rb

@@ -755,6 +755,18 @@ class QtBinding_TestClass < TestBase
 
   end
 
+  def test_54
+
+    # issue #1029 (Crash for QBrush passed to setData)
+
+    item = RBA::QTreeWidgetItem::new
+    item.setBackground(0, RBA::QBrush::new(RBA::QColor::new(0xFF, 0xFF, 0x00)))
+    assert_equal(item.background(0).color.red, 255)
+    assert_equal(item.background(0).color.green, 255)
+    assert_equal(item.background(0).color.blue, 0)
+
+  end
+
 end 
 
 load("test_epilogue.rb")