GNOME Bugzilla – Bug 547306
Hard to get a RefPtr<> to this.
Last modified: 2018-05-22 12:09:06 UTC
Hi, I'm writing a button actor, that inherits from clutter::actor, and I want to use an effectTemplate when the button is pressed. I wrote the following code in my function button_actor::real_released() m_press_effect->fade(this, 0x44,sigc::bind(sigc::ptr_fun(do_nothing), m_press_effect)); m_press_effect is of type Clutter::EffectTemplate, and the problem is that fade's first parameter is not actor pointer, but a actor smart pointer Glib::RefPtr<Clutter::Actor>). I cannot wrap this with smart pointer like : m_press_effect->fade(Glib::RefPtr<Clutter::Actor>(this), 0x44,sigc::bind(sigc::ptr_fun(do_nothing), m_press_effect)); because at the end of the function, the reference count will go to 0, and it will do 'delete this' which is very bad. Is there any other way to write this? I wonder how GTKmm and other mm projects solve similar problems like this. Another small problem, is that some functions in clutter can get NULL, but in case you use reference, you cannot pass NULL.
I agree that this is not obvious and it is ugly, but you can do this: Glib::RefPtr<MyClass> refptr_this = Glib::RefPtr<MyClass>(this); refptr->reference(); I wonder how we could make this easier.
Wouldn't this solution create a memory leak?
No.(In reply to comment #2) > Wouldn't this solution create a memory leak? No, that initial reference is necessary, because the constructor doesn't take a reference (because it's meant for use by create() functions that know whether they need to do this. Remember it's not a general purpose smartpointer.) The reference will be unreferenced by the destructor.
Not obvious and ugly? Now that's an understatement. I think the RefPtr<> ctor should acquire a reference by default, and I'm going to propose that change (and many others) for gtkmm 3.
Created attachment 244979 [details] [review] patch: RefPtr: Add constructor RefPtr(T_CppObject* pCppObject, bool take_ref). wrap() functions have a take_copy parameter. We could add a RefPtr constructor with a similar parameter, take_ref.
Created attachment 244980 [details] testcase.cc Test case, testing the proposed constructor.
C++11's std::shared_ptr<> deals with this problem by allowing the contained class to derive from std::enable_shared_by_this so you can then call shared_by_this(). http://en.cppreference.com/w/cpp/memory/enable_shared_from_this Maybe we should consider something similar, for consistency. Of course, it would also be nice if we could somehow replace Glib::RefPtr with std::sharedptr<>, but I don't see any way to specify any extra class to do the underlying cleverness.
I'm making this bug dependent on bug #755037 , because this would be simplified by us just using std::shared_ptr instead of RefPtr.
Created attachment 349287 [details] [review] 0001-TextMark-Avoid-creating-a-RefPtr-to-this.patch
Sorry, I put that patch on the wrong bug.
Isn't this obsolete now? glibmm master uses std::shared_ptr as Glib::RefPtr, so that side seems covered. And the stable branch presumably won't add such a change.
The problem with Glib::RefPtr(this) is hardly solved by using std::shared_ptr. See several comments in bug 775037, such as comments 26, 27, 28, 37, 39. Murray had some trouble removing all internal RefPtr(this) in gtkmm. Should this bug be closed as Wontfix? Or add template <class T_CppObject> RefPtr<T_CppObject> make_refptr_for_this(T_CppObject* object) { object->reference(); return RefPtr<T_CppObject>(object, &RefPtrDeleter<T_CppObject>); } analogous to Glib::make_refptr_for_instance()?
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glibmm/issues/8.