vitorsousa pushed a commit to branch master.

http://git.enlightenment.org/core/efl.git/commit/?id=afb66463937a8d08e2bf1d538641a9520ba457d6

commit afb66463937a8d08e2bf1d538641a9520ba457d6
Author: Vitor Sousa <vitorsousasi...@gmail.com>
Date:   Wed Mar 16 19:46:22 2016 -0300

    efl js: Add clean up callbacks to deallocate memory used in v8::External
    
    Add several garbage collector callbacks for cleaning allocated C and C++
    data used inside v8:External objects.
    
    Fix eo_unref of already freed object in eo_js_construct_from_eo.hh.
    
    Ensure all structs are allocated with malloc.
    
    Add test for garbage collection.
    Had to created .sh script because shebang clause do not support multiple
    arguments.
---
 src/Makefile_Eolian_Js.am                          |  4 +-
 src/bindings/js/eina_js/eina_js_compatibility.hh   |  5 +-
 src/bindings/js/eina_js/eina_js_container.cc       | 16 +++-
 src/bindings/js/eina_js/eina_js_container.hh       |  5 ++
 .../js/eina_js/eina_js_get_value_from_c.hh         | 89 +++++++++++++++++-----
 src/bindings/js/eina_js/eina_js_value.cc           |  3 +
 src/bindings/js/eo_js/eo_js_construct_from_eo.hh   |  1 +
 src/bindings/js/eo_js/eo_js_event.hh               |  1 +
 src/bindings/js/eo_js/eo_js_struct.hh              |  6 +-
 src/tests/eolian_js/eolian_js_suite.js             | 16 ++++
 src/tests/eolian_js/eolian_js_suite.sh             |  4 +
 11 files changed, 123 insertions(+), 27 deletions(-)

diff --git a/src/Makefile_Eolian_Js.am b/src/Makefile_Eolian_Js.am
index 32cf893..b15fa87 100644
--- a/src/Makefile_Eolian_Js.am
+++ b/src/Makefile_Eolian_Js.am
@@ -25,11 +25,11 @@ include Makefile_Eolian_Js_Helper.am
 if EFL_ENABLE_TESTS
 if HAVE_NODEJS
 
-TESTS += tests/eolian_js/eolian_js_suite.js
+TESTS += tests/eolian_js/eolian_js_suite.sh
 
 check_LTLIBRARIES += tests/eolian_js/libeolian_js_suite.la
 
-tests/eolian_js/eolian_js_suite.js: tests/eolian_js/eolian_js_suite_mod.node
+tests/eolian_js/eolian_js_suite.sh: tests/eolian_js/eolian_js_suite_mod.node
 tests/eolian_js/eolian_js_suite_mod.node: tests/eolian_js/libeolian_js_suite.la
        $(AM_V_CP)$(CP) 
$(top_builddir)/src/tests/eolian_js/.libs/libeolian_js_suite.so 
$(top_builddir)/src/tests/eolian_js/eolian_js_suite_mod.node
 
diff --git a/src/bindings/js/eina_js/eina_js_compatibility.hh 
b/src/bindings/js/eina_js/eina_js_compatibility.hh
index fb36e4e..7a46714 100644
--- a/src/bindings/js/eina_js/eina_js_compatibility.hh
+++ b/src/bindings/js/eina_js/eina_js_compatibility.hh
@@ -927,8 +927,9 @@ compatibility_return_type 
cast_function(compatibility_callback_info_type args)
       char* class_name = *str;
 
       auto ctor = ::efl::eina::js::get_class_constructor(class_name);
-      return compatibility_return
-        (new_v8_external_instance(ctor, ::eo_ref(eo), isolate), args);
+      auto obj = new_v8_external_instance(ctor, ::eo_ref(eo), isolate);
+      efl::eina::js::make_weak(isolate, obj, [eo]{ ::eo_unref(eo); });
+      return compatibility_return(obj, args);
     }
   else
     {
diff --git a/src/bindings/js/eina_js/eina_js_container.cc 
b/src/bindings/js/eina_js/eina_js_container.cc
index 60c53e0..82246c5 100644
--- a/src/bindings/js/eina_js/eina_js_container.cc
+++ b/src/bindings/js/eina_js/eina_js_container.cc
@@ -54,11 +54,14 @@ v8::Local<v8::Value> concat(eina_container_base& lhs, 
v8::Isolate* isolate, v8::
             , &typeinfo_rhs = typeid(rhs);
           if(!typeinfo_lhs.before(typeinfo_rhs) && 
!typeinfo_rhs.before(typeinfo_lhs))
             {
+              auto ptr = rhs.concat(lhs);
               v8::Handle<v8::Value> a[] =
-                {efl::eina::js::compatibility_new<v8::External>(isolate, 
rhs.concat(lhs))};
+                {efl::eina::js::compatibility_new<v8::External>(isolate, ptr)};
               assert(!!*instance_templates[lhs.get_container_type()].handle());
               v8::Local<v8::Object> result =
                 
instance_templates[lhs.get_container_type()].handle()->NewInstance(1, a);
+              if (ptr)
+                efl::eina::js::make_weak(isolate, result, [ptr]{ delete ptr; 
});
               return result;
             }
           else
@@ -95,9 +98,12 @@ v8::Local<v8::Value> slice(eina_container_base& self, 
v8::Isolate* isolate, v8::
   else
     return v8::Undefined(isolate);
 
-  v8::Handle<v8::Value> a[] = 
{efl::eina::js::compatibility_new<v8::External>(isolate, self.slice(i, j))};
+  auto ptr = self.slice(i, j);
+  v8::Handle<v8::Value> a[] = 
{efl::eina::js::compatibility_new<v8::External>(isolate, ptr)};
   v8::Local<v8::Object> result = 
instance_templates[self.get_container_type()].handle()
     ->NewInstance(1, a);
+  if (ptr)
+    efl::eina::js::make_weak(isolate, result, [ptr]{ delete ptr; });
   return result;
 }
 
@@ -154,6 +160,7 @@ compatibility_return_type 
new_eina_list_internal(compatibility_callback_info_typ
           eina_container_base* p = new eina_list<int>;
           compatibility_set_pointer_internal_field
             (args.This(), 0, dynamic_cast<void*>(p));
+          efl::eina::js::make_weak(args.GetIsolate(), args.This(), [p]{ delete 
p; });
         }
       else
         {
@@ -182,6 +189,7 @@ compatibility_return_type 
new_eina_list(compatibility_callback_info_type args)
           eina_container_base* p = new eina_list<int>;
           compatibility_set_pointer_internal_field
             (args.This(), 0, dynamic_cast<void*>(p));
+          efl::eina::js::make_weak(args.GetIsolate(), args.This(), [p]{ delete 
p; });
           return compatibility_return();
         }
         else if (args.Length() == 1 && args[0]->IsString())
@@ -195,6 +203,7 @@ compatibility_return_type 
new_eina_list(compatibility_callback_info_type args)
           }
           compatibility_set_pointer_internal_field
             (args.This(), 0, dynamic_cast<void*>(p));
+          efl::eina::js::make_weak(args.GetIsolate(), args.This(), [p]{ delete 
p; });
           return compatibility_return();
         }
     }
@@ -212,6 +221,7 @@ compatibility_return_type 
new_eina_array_internal(compatibility_callback_info_ty
           eina_container_base* p = new eina_array<int>;
           compatibility_set_pointer_internal_field
             (args.This(), 0, dynamic_cast<void*>(p));
+          efl::eina::js::make_weak(args.GetIsolate(), args.This(), [p]{ delete 
p; });
         }
       else
         {
@@ -240,6 +250,7 @@ compatibility_return_type 
new_eina_array(compatibility_callback_info_type args)
           eina_container_base* p = new eina_array<int>;
           compatibility_set_pointer_internal_field
             (args.This(), 0, dynamic_cast<void*>(p));
+          efl::eina::js::make_weak(args.GetIsolate(), args.This(), [p]{ delete 
p; });
           return compatibility_return();
         }
         else if (args.Length() == 1 && args[0]->IsString())
@@ -253,6 +264,7 @@ compatibility_return_type 
new_eina_array(compatibility_callback_info_type args)
           }
           compatibility_set_pointer_internal_field
             (args.This(), 0, dynamic_cast<void*>(p));
+          efl::eina::js::make_weak(args.GetIsolate(), args.This(), [p]{ delete 
p; });
           return compatibility_return();
         }
     }
diff --git a/src/bindings/js/eina_js/eina_js_container.hh 
b/src/bindings/js/eina_js/eina_js_container.hh
index 21343fd..62fe22c 100644
--- a/src/bindings/js/eina_js/eina_js_container.hh
+++ b/src/bindings/js/eina_js/eina_js_container.hh
@@ -273,6 +273,11 @@ struct eina_container_common : 
eina_container_type_specific<C, typename C::value
        (eina::js::compatibility_new<v8::String>(isolate, "Indexed attribution 
was not implemented.")));
     return v8::Undefined(isolate);
   }
+
+  typename C::native_handle_type release_native_handle()
+  {
+    return _container.release_native_handle();
+  }
   C _container;
   typedef C container_type;
 };
diff --git a/src/bindings/js/eina_js/eina_js_get_value_from_c.hh 
b/src/bindings/js/eina_js/eina_js_get_value_from_c.hh
index 204c497..653a96e 100644
--- a/src/bindings/js/eina_js/eina_js_get_value_from_c.hh
+++ b/src/bindings/js/eina_js/eina_js_get_value_from_c.hh
@@ -143,7 +143,9 @@ inline v8::Local<v8::Value>
 get_value_from_c(Eo* v, v8::Isolate* isolate, const char* class_name)
 {
   auto ctor = ::efl::eina::js::get_class_constructor(class_name);
-  return new_v8_external_instance(ctor, v, isolate);
+  auto obj = new_v8_external_instance(ctor, ::eo_ref(v), isolate);
+  efl::eina::js::make_weak(isolate, obj, [v]{ ::eo_unref(v); });
+  return obj;
 }
 
 inline v8::Local<v8::Value>
@@ -151,23 +153,32 @@ get_value_from_c(const Eo* v, v8::Isolate* isolate, const 
char* class_name)
 {
   // TODO: implement const objects?
   auto ctor = ::efl::eina::js::get_class_constructor(class_name);
-  return new_v8_external_instance(ctor, const_cast<Eo*>(v), isolate);
+  auto obj = new_v8_external_instance(ctor, ::eo_ref(v), isolate);
+  efl::eina::js::make_weak(isolate, obj, [v]{ ::eo_unref(v); });
+  return obj;
 }
 
 template <typename T>
 inline v8::Local<v8::Value>
-get_value_from_c(struct_ptr_tag<T> v, v8::Isolate* isolate, const char* 
class_name)
+get_value_from_c(struct_ptr_tag<T*> v, v8::Isolate* isolate, const char* 
class_name)
 {
-  // TODO: implement const structs?
+  using nonconst_type = typename std::remove_const<T>::type;
+  bool own = false; // TODO: handle ownership
+  auto ptr = const_cast<nonconst_type*>(v.value);
   auto ctor = ::efl::eina::js::get_class_constructor(class_name);
-  return new_v8_external_instance(ctor, const_cast<typename 
std::remove_const<T>::type>(v.value), isolate);
+  auto obj = new_v8_external_instance(ctor, ptr, isolate);
+  if (own)
+    efl::eina::js::make_weak(isolate, obj, [ptr]{ free(ptr); });
+  return obj;
 }
 
 template <typename T>
 inline v8::Local<v8::Value>
 get_value_from_c(struct_tag<T> v, v8::Isolate* isolate, const char* class_name)
 {
-  return get_value_from_c(struct_ptr_tag<T*>{new T(v.value)}, isolate, 
class_name);
+  T* s = static_cast<T*>(malloc(sizeof(T)));
+  *s = v.value;
+  return get_value_from_c(struct_ptr_tag<T*>{s}, isolate, class_name);
 }
 
 template <typename T, typename K>
@@ -175,8 +186,16 @@ inline v8::Local<v8::Value>
 get_value_from_c(efl::eina::js::complex_tag<Eina_Accessor *, T, K> v, 
v8::Isolate* isolate, const char*)
 {
   using wrapped_type = typename container_wrapper<T>::type;
+  bool own = false; // TODO: handle ownership
   auto a = new ::efl::eina::accessor<wrapped_type>{v.value};
-  return export_accessor<T>(*a , isolate, K::class_name());
+  auto obj = export_accessor<T>(*a , isolate, K::class_name());
+  efl::eina::js::make_weak(isolate, obj, [a, own]
+    {
+       if (!own)
+         a->release_native_handle();
+       delete a;
+    });
+  return obj;
 }
 
 template <typename T, typename K>
@@ -191,18 +210,34 @@ template <typename T, typename K>
 inline v8::Local<v8::Value>
 get_value_from_c(efl::eina::js::complex_tag<Eina_Array *, T, K> v, 
v8::Isolate* isolate, const char*)
 {
-  // TODO: use unique_ptr for eina_array to avoid leak ?
-  auto o = new ::efl::eina::js::range_eina_array<T, K>(v.value);
+  bool own = false; // TODO: handle ownership
+  auto o = new ::efl::eina::js::eina_array<T, K>(v.value);
   auto ctor = get_array_instance_template();
-  return new_v8_external_instance(ctor, o, isolate);
+  auto obj = new_v8_external_instance(ctor, o, isolate);
+  efl::eina::js::make_weak(isolate, obj, [o, own]
+    {
+       if (!own)
+         o->release_native_handle();
+       delete o;
+    });
+  return obj;
 }
 
 template <typename T, typename K>
 inline v8::Local<v8::Value>
-get_value_from_c(efl::eina::js::complex_tag<const Eina_Array *, T, K> v, 
v8::Isolate* isolate, const char* class_name)
+get_value_from_c(efl::eina::js::complex_tag<const Eina_Array *, T, K> v, 
v8::Isolate* isolate, const char*)
 {
-  // TODO: implement const array?
-  return get_value_from_c(efl::eina::js::complex_tag<Eina_Array *, T, 
K>{const_cast<Eina_Array*>(v.value)}, isolate, class_name);
+  bool own = false; // TODO: handle ownership
+  auto o = new ::efl::eina::js::range_eina_array<T, 
K>(const_cast<Eina_Array*>(v.value));
+  auto ctor = get_array_instance_template();
+  auto obj = new_v8_external_instance(ctor, o, isolate);
+  efl::eina::js::make_weak(isolate, obj, [o, own]
+    {
+       if (!own)
+         o->release_native_handle();
+       delete o;
+    });
+  return obj;
 }
 
 template <typename T, typename K>
@@ -245,18 +280,34 @@ template <typename T, typename K>
 inline v8::Local<v8::Value>
 get_value_from_c(efl::eina::js::complex_tag<Eina_List *, T, K> v, v8::Isolate* 
isolate, const char*)
 {
-  // TODO: ensure eina_list ownership ???
-  auto o = new ::efl::eina::js::range_eina_list<T, K>(v.value);
+  bool own = false; // TODO: handle ownership
+  auto o = new ::efl::eina::js::eina_list<T, K>(v.value);
   auto ctor = get_list_instance_template();
-  return new_v8_external_instance(ctor, o, isolate);
+  auto obj = new_v8_external_instance(ctor, o, isolate);
+  efl::eina::js::make_weak(isolate, obj, [o, own]
+    {
+       if (!own)
+         o->release_native_handle();
+       delete o;
+    });
+  return obj;
 }
 
 template <typename T, typename K>
 inline v8::Local<v8::Value>
-get_value_from_c(efl::eina::js::complex_tag<const Eina_List *, T, K> v, 
v8::Isolate* isolate, const char* class_name)
+get_value_from_c(efl::eina::js::complex_tag<const Eina_List *, T, K> v, 
v8::Isolate* isolate, const char*)
 {
-  // TODO: implement const list?
-  return get_value_from_c(efl::eina::js::complex_tag<Eina_List *, T, 
K>{const_cast<Eina_List*>(v.value)}, isolate, class_name);
+  bool own = false; // TODO: handle ownership
+  auto o = new ::efl::eina::js::range_eina_list<T, 
K>(const_cast<Eina_List*>(v.value));
+  auto ctor = get_list_instance_template();
+  auto obj = new_v8_external_instance(ctor, o, isolate);
+  efl::eina::js::make_weak(isolate, obj, [o, own]
+    {
+       if (!own)
+         o->release_native_handle();
+       delete o;
+    });
+  return obj;
 }
 
 
diff --git a/src/bindings/js/eina_js/eina_js_value.cc 
b/src/bindings/js/eina_js/eina_js_value.cc
index 5b5b822..c7377ae 100644
--- a/src/bindings/js/eina_js/eina_js_value.cc
+++ b/src/bindings/js/eina_js/eina_js_value.cc
@@ -54,6 +54,9 @@ compatibility_return_type 
eina_value_constructor(compatibility_callback_info_typ
     std::unique_ptr<value>
       ptr(new value(value_cast<value>(args[0])));
     compatibility_set_pointer_internal_field(args.This(), 0, ptr.get());
+    auto v = ptr.get();
+    if (v)
+      efl::eina::js::make_weak(isolate, args.This(), [v]{ delete v; });
     ptr.release();
   } catch(const std::bad_cast &e) {
     v8::Local<v8::Object> je = compatibility_new<v8::Object>(isolate);
diff --git a/src/bindings/js/eo_js/eo_js_construct_from_eo.hh 
b/src/bindings/js/eo_js/eo_js_construct_from_eo.hh
index 287aaa7..8008d16 100644
--- a/src/bindings/js/eo_js/eo_js_construct_from_eo.hh
+++ b/src/bindings/js/eo_js/eo_js_construct_from_eo.hh
@@ -20,6 +20,7 @@ inline eina::js::compatibility_return_type 
construct_from_eo(eina::js::compatibi
     {
       Eo* eo = static_cast<Eo*>(v8::External::Cast(*args[0])->Value());
       args.This()->SetInternalField(0, args[0]);
+      ::eo_ref(eo);
       efl::eina::js::make_weak(args.GetIsolate(), args.This(), [eo] { 
eo_unref(eo); });
       return eina::js::compatibility_return();
     }
diff --git a/src/bindings/js/eo_js/eo_js_event.hh 
b/src/bindings/js/eo_js/eo_js_event.hh
index 280aeac..b589e5d 100644
--- a/src/bindings/js/eo_js/eo_js_event.hh
+++ b/src/bindings/js/eo_js/eo_js_event.hh
@@ -100,6 +100,7 @@ inline eina::js::compatibility_return_type 
on_event(eina::js::compatibility_call
           event_callback_information* i = new event_callback_information
             {event, {isolate, eina::js::compatibility_cast<v8::Function>(f)}};
           eo_event_callback_add(eo, event->event, event->event_callback, i);
+          efl::eina::js::make_weak(isolate, self, [i]{ delete i; });
         }
       else
         {
diff --git a/src/bindings/js/eo_js/eo_js_struct.hh 
b/src/bindings/js/eo_js/eo_js_struct.hh
index 731f9d0..eba0c74 100644
--- a/src/bindings/js/eo_js/eo_js_struct.hh
+++ b/src/bindings/js/eo_js/eo_js_struct.hh
@@ -42,8 +42,10 @@ eina::js::compatibility_return_type 
new_struct(eina::js::compatibility_callback_
 
   if(args.Length() == 0)
     {
-      S* p = new S{};
-      eina::js::compatibility_set_pointer_internal_field(args.This(), 0, 
static_cast<void*>(p));
+      void* p = std::malloc(sizeof(S));
+      std::memset(p, 0, sizeof(S));
+      eina::js::compatibility_set_pointer_internal_field(args.This(), 0, p);
+      efl::eina::js::make_weak(args.GetIsolate(), args.This(), [p]{ free(p); 
});
     }
   else
     {
diff --git a/src/tests/eolian_js/eolian_js_suite.js 
b/src/tests/eolian_js/eolian_js_suite.js
index 66e5596..2c45eb3 100755
--- a/src/tests/eolian_js/eolian_js_suite.js
+++ b/src/tests/eolian_js/eolian_js_suite.js
@@ -795,6 +795,22 @@ startTest("method_iterator_of_structs", function() {
 //   assert(a[0] === 42);
 // });
 
+// Garbage Collection //
+startTest("gc_object", function() {
+  var freed = false;
+  (function() {
+    var obj = new TestObject(null);
+    obj.on("del", function() {
+           printInfo('Object destructed')
+           freed = true;
+    });
+  }());
+  printInfo('going to garbage collect');
+  global.gc();
+  printInfo('is object destructed?');
+  assert(freed);
+});
+
 startTest("new Constructor_Method_Class", function() {
   var obj = new ConstructorMethodClass(null, 5, 10.0);
   assert(obj);
diff --git a/src/tests/eolian_js/eolian_js_suite.sh 
b/src/tests/eolian_js/eolian_js_suite.sh
new file mode 100755
index 0000000..6b78a50
--- /dev/null
+++ b/src/tests/eolian_js/eolian_js_suite.sh
@@ -0,0 +1,4 @@
+#!/bin/sh
+
+BASEDIR=$(dirname $0)
+/usr/bin/env node --expose-gc $BASEDIR/eolian_js_suite.js

-- 


Reply via email to