From 8ab62d702c4a97579a7afbb455982c4d006ac911 Mon Sep 17 00:00:00 2001
From: bubnikv <bubnikv@gmail.com>
Date: Wed, 18 Apr 2018 18:06:07 +0200
Subject: [PATCH] Fixed memory leak of TabIface. Added documentation of the XS
 interface on how the Ref<> and Clone<> wrappers work.

---
 xs/src/xsinit.h     | 26 ++++++++++++++++++++++++--
 xs/xsp/GUI.xsp      |  2 +-
 xs/xsp/typemap.xspt |  1 -
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/xs/src/xsinit.h b/xs/src/xsinit.h
index 96c4b74d7..9cb60384f 100644
--- a/xs/src/xsinit.h
+++ b/xs/src/xsinit.h
@@ -85,8 +85,12 @@ namespace Slic3r {
     
 template<class T>
 struct ClassTraits { 
+    // Name of a Perl alias of a C++ class type, owned by Perl, reference counted.
     static const char* name;
-    static const char* name_ref; 
+    // Name of a Perl alias of a C++ class type, owned by the C++ code.
+    // The references shall be enumerated at the end of XS.pm, where the desctructor is undefined with sub DESTROY {},
+    // so Perl will never delete the object instance.
+    static const char* name_ref;
 };
 
 // use this for typedefs for which the forward prototype
@@ -99,11 +103,16 @@ struct ClassTraits {
     class cname;                                                                     \
     __REGISTER_CLASS(cname, perlname);
 
+// Return Perl alias to a C++ class name.
 template<class T>
 const char* perl_class_name(const T*) { return ClassTraits<T>::name; }
+// Return Perl alias to a C++ class name, suffixed with ::Ref.
+// Such a C++ class instance will not be destroyed by Perl, the instance destruction is left to the C++ code.
 template<class T>
 const char* perl_class_name_ref(const T*) { return ClassTraits<T>::name_ref; }
 
+// Mark the Perl SV (Scalar Value) as owning a "blessed" pointer to an object reference.
+// Perl will never release the C++ instance.
 template<class T>
 SV* perl_to_SV_ref(T &t) {
     SV* sv = newSV(0);
@@ -111,6 +120,8 @@ SV* perl_to_SV_ref(T &t) {
     return sv;
 }
 
+// Mark the Perl SV (Scalar Value) as owning a "blessed" pointer to an object instance.
+// Perl will own the C++ instance, therefore it will also release it.
 template<class T>
 SV* perl_to_SV_clone_ref(const T &t) {
     SV* sv = newSV(0);
@@ -118,6 +129,8 @@ SV* perl_to_SV_clone_ref(const T &t) {
     return sv;
 }
 
+// Reference wrapper to provide a C++ instance to Perl while keeping Perl from destroying the instance.
+// The instance is created temporarily by XS.cpp just to provide Perl with a CLASS name and a object instance pointer.
 template <class T> 
 class Ref {
     T* val;
@@ -125,10 +138,15 @@ public:
     Ref() : val(NULL) {}
     Ref(T* t) : val(t) {}
     Ref(const T* t) : val(const_cast<T*>(t)) {}
+    // Called by XS.cpp to convert the referenced object instance to a Perl SV, before it is blessed with the name
+    // returned by CLASS()
     operator T*() const { return val; }
+    // Name to bless the Perl SV with. The name ends with a "::Ref" suffix to keep Perl from destroying the object instance.
     static const char* CLASS() { return ClassTraits<T>::name_ref; }
 };
-  
+
+// Wrapper to clone a C++ object instance before passing it to Perl for ownership.
+// This wrapper instance is created temporarily by XS.cpp to provide Perl with a CLASS name and a object instance pointer.
 template <class T>
 class Clone {
     T* val;
@@ -136,7 +154,11 @@ public:
     Clone() : val(NULL) {}
     Clone(T* t) : val(new T(*t)) {}
     Clone(const T& t) : val(new T(t)) {}
+    // Called by XS.cpp to convert the cloned object instance to a Perl SV, before it is blessed with the name
+    // returned by CLASS()
     operator T*() const { return val; }
+    // Name to bless the Perl SV with. If there is a destructor registered in the XSP file for this class, then Perl will
+    // call this destructor when the reference counter of this SV drops to zero.
     static const char* CLASS() { return ClassTraits<T>::name; }
 };
 
diff --git a/xs/xsp/GUI.xsp b/xs/xsp/GUI.xsp
index 1376ff164..406d71828 100644
--- a/xs/xsp/GUI.xsp
+++ b/xs/xsp/GUI.xsp
@@ -39,7 +39,7 @@ void add_debug_menu(SV *ui, int event_language_change)
 void create_preset_tabs(bool no_controller, int event_value_change, int event_presets_changed)
     %code%{ Slic3r::GUI::create_preset_tabs(no_controller, event_value_change, event_presets_changed); %};
 
-Ref<TabIface> get_preset_tab(char *name)
+TabIface* get_preset_tab(char *name)
     %code%{ RETVAL=Slic3r::GUI::get_preset_tab_iface(name); %};
 
 bool load_language()
diff --git a/xs/xsp/typemap.xspt b/xs/xsp/typemap.xspt
index 0214a158d..797f3c19f 100644
--- a/xs/xsp/typemap.xspt
+++ b/xs/xsp/typemap.xspt
@@ -211,7 +211,6 @@
 %typemap{PresetHints*};
 %typemap{Ref<PresetHints>}{simple};
 %typemap{TabIface*};
-%typemap{Ref<TabIface>}{simple};
 
 %typemap{PrintRegionPtrs*};
 %typemap{PrintObjectPtrs*};