This brings over the memory leak fixes for pointers to structs with a %extend destructor from my Neha fork. The generator was not generating and connecting the needed code for the requested destructor to the v8 dtor finalizer.
I did not realize this branch has some JavaScriptCore stuff in it too. Unfortunately, it seems to have its own unique problems (like creating C++ files when it should be generating C files). My changes are targeted for v8, and I don't think my JSCore changes fully reach in this JSCore implementation so more work would need to be done to get this branch working. I think my Neha fork is in better shape at the moment.
Also, I did port over the 'NULL out the dtor function pointer' in the %nodefaultdtor fix to v8.
Usage case:
struct MyData {
%extend {
~MyData() {
FreeData($self);
}
}
};
%newobject CreateData;
struct MyData* CreateData(void);
%delobject FreeData;
void FreeData(struct MyData* the_data);
where the use case is something like:
var my_data = example.CreateData();
my_data = null;
This commit is contained in:
parent
caa92740d3
commit
ed729f7d3a
3 changed files with 137 additions and 4 deletions
|
|
@ -113,6 +113,25 @@ void $jswrapper(JSObjectRef thisObject)
|
||||||
}
|
}
|
||||||
%}
|
%}
|
||||||
|
|
||||||
|
/* -----------------------------------------------------------------------------
|
||||||
|
* js_dtor: template for a destructor wrapper
|
||||||
|
* - $jsmangledname: mangled class name
|
||||||
|
* - $jstype: class type
|
||||||
|
* - ${destructor_action}: The custom destructor action to invoke.
|
||||||
|
* ----------------------------------------------------------------------------- */
|
||||||
|
%fragment ("js_dtoroverride", "templates")
|
||||||
|
%{
|
||||||
|
void $jswrapper(JSObjectRef thisObject)
|
||||||
|
{
|
||||||
|
SWIG_PRV_DATA* t = (SWIG_PRV_DATA*) JSObjectGetPrivate(thisObject);
|
||||||
|
if(t && t->swigCMemOwn) {
|
||||||
|
$jstype* arg1 = ($jstype*)t->swigCObject;
|
||||||
|
${destructor_action}
|
||||||
|
}
|
||||||
|
if(t) free(t);
|
||||||
|
}
|
||||||
|
%}
|
||||||
|
|
||||||
/* -----------------------------------------------------------------------------
|
/* -----------------------------------------------------------------------------
|
||||||
* js_getter: template for getter function wrappers
|
* js_getter: template for getter function wrappers
|
||||||
* - $jswrapper: wrapper function name
|
* - $jswrapper: wrapper function name
|
||||||
|
|
|
||||||
|
|
@ -114,13 +114,32 @@ fail:
|
||||||
void $jswrapper(v8::Persistent< v8::Value > object, void *parameter) {
|
void $jswrapper(v8::Persistent< v8::Value > object, void *parameter) {
|
||||||
SWIGV8_Proxy* proxy = (SWIGV8_Proxy*) parameter;
|
SWIGV8_Proxy* proxy = (SWIGV8_Proxy*) parameter;
|
||||||
if(proxy->swigCMemOwn && proxy->swigCObject) {
|
if(proxy->swigCMemOwn && proxy->swigCObject) {
|
||||||
std::cout << "Deleting wrapped instance: " << proxy->info->name << std::endl;
|
// std::cout << "Deleting wrapped instance: " << proxy->info->name << std::endl;
|
||||||
$jsfree proxy->swigCObject;
|
$jsfree proxy->swigCObject;
|
||||||
}
|
}
|
||||||
delete proxy;
|
delete proxy;
|
||||||
}
|
}
|
||||||
%}
|
%}
|
||||||
|
|
||||||
|
/* -----------------------------------------------------------------------------
|
||||||
|
* js_dtoroverride: template for a destructor wrapper
|
||||||
|
* - $jsmangledname: mangled class name
|
||||||
|
* - $jstype: class type
|
||||||
|
* - ${destructor_action}: The custom destructor action to invoke.
|
||||||
|
* ----------------------------------------------------------------------------- */
|
||||||
|
%fragment ("js_dtoroverride", "templates")
|
||||||
|
%{
|
||||||
|
void $jswrapper(v8::Persistent< v8::Value > object, void *parameter) {
|
||||||
|
SWIGV8_Proxy* proxy = (SWIGV8_Proxy*) parameter;
|
||||||
|
if(proxy->swigCMemOwn && proxy->swigCObject) {
|
||||||
|
// std::cout << "Deleting wrapped instance: " << proxy->info->name << std::endl;
|
||||||
|
$jstype arg1 = ($jstype)t->swigCObject;
|
||||||
|
${destructor_action}
|
||||||
|
}
|
||||||
|
delete proxy;
|
||||||
|
}
|
||||||
|
%}
|
||||||
|
|
||||||
/* -----------------------------------------------------------------------------
|
/* -----------------------------------------------------------------------------
|
||||||
* js_getter: template for getter function wrappers
|
* js_getter: template for getter function wrappers
|
||||||
* - $jswrapper: wrapper function name
|
* - $jswrapper: wrapper function name
|
||||||
|
|
|
||||||
|
|
@ -886,7 +886,6 @@ int JSEmitter::emitCtor(Node *n) {
|
||||||
|
|
||||||
int JSEmitter::emitDtor(Node *n) {
|
int JSEmitter::emitDtor(Node *n) {
|
||||||
|
|
||||||
Template t_dtor = getTemplate("js_dtor");
|
|
||||||
String *wrap_name = Swig_name_wrapper(Getattr(n, "sym:name"));
|
String *wrap_name = Swig_name_wrapper(Getattr(n, "sym:name"));
|
||||||
|
|
||||||
SwigType *type = state.clazz(TYPE);
|
SwigType *type = state.clazz(TYPE);
|
||||||
|
|
@ -894,20 +893,116 @@ int JSEmitter::emitDtor(Node *n) {
|
||||||
String *ctype = SwigType_lstr(p_classtype, "");
|
String *ctype = SwigType_lstr(p_classtype, "");
|
||||||
String *free = NewString("");
|
String *free = NewString("");
|
||||||
|
|
||||||
|
// (Taken from JSCore implementation.)
|
||||||
|
/* The if (Extend) block was taken from the Ruby implementation.
|
||||||
|
* The problem is that in the case of an %extend to create a destructor for a struct to coordinate automatic memory cleanup with the Javascript collector,
|
||||||
|
* the swig function was not being generated. More specifically:
|
||||||
|
struct MyData {
|
||||||
|
%extend {
|
||||||
|
~MyData() {
|
||||||
|
FreeData($self);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
};
|
||||||
|
%newobject CreateData;
|
||||||
|
struct MyData* CreateData(void);
|
||||||
|
%delobject FreeData;
|
||||||
|
void FreeData(struct MyData* the_data);
|
||||||
|
|
||||||
|
where the use case is something like:
|
||||||
|
var my_data = example.CreateData();
|
||||||
|
my_data = null;
|
||||||
|
|
||||||
|
This function was not being generated:
|
||||||
|
SWIGINTERN void delete_MyData(struct MyData *self){
|
||||||
|
FreeData(self);
|
||||||
|
}
|
||||||
|
|
||||||
|
I don't understand fully why it wasn't being generated. It just seems to happen in the Lua generator.
|
||||||
|
There is a comment about staticmemberfunctionHandler having an inconsistency and I tracked down dome of the SWIGINTERN void delete_*
|
||||||
|
code to that function in the Language base class.
|
||||||
|
The Ruby implementation seems to have an explicit check for if(Extend) and explicitly generates the code, so that's what I'm doing here.
|
||||||
|
The Ruby implementation does other stuff which I omit.
|
||||||
|
*/
|
||||||
|
if (Extend) {
|
||||||
|
String *wrap = Getattr(n, "wrap:code");
|
||||||
|
if (wrap) {
|
||||||
|
Printv(f_wrappers, wrap, NIL);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
// HACK: this is only for the v8 emitter. maybe set an attribute wrap:action of node
|
// HACK: this is only for the v8 emitter. maybe set an attribute wrap:action of node
|
||||||
// TODO: generate dtors more similar to other wrappers
|
// TODO: generate dtors more similar to other wrappers
|
||||||
|
// EW: I think this is wrong. delete should only be used when new was used to create. If malloc was used, free needs to be used.
|
||||||
if(SwigType_isarray(type)) {
|
if(SwigType_isarray(type)) {
|
||||||
Printf(free, "delete [] (%s)", ctype);
|
Printf(free, "delete [] (%s)", ctype);
|
||||||
} else {
|
} else {
|
||||||
Printf(free, "delete (%s)", ctype);
|
Printf(free, "delete (%s)", ctype);
|
||||||
}
|
}
|
||||||
|
|
||||||
state.clazz(DTOR, wrap_name);
|
String* destructor_action = Getattr(n, "wrap:action");
|
||||||
t_dtor.replace(T_NAME_MANGLED, state.clazz(NAME_MANGLED))
|
// Adapted from the JSCore implementation.
|
||||||
|
/* The next challenge is to generate the correct finalize function for JavaScriptCore to call.
|
||||||
|
Originally, it would use this fragment from javascriptcode.swg
|
||||||
|
%fragment ("JS_destructordefn", "templates")
|
||||||
|
%{
|
||||||
|
void _wrap_${classname_mangled}_finalize(JSObjectRef thisObject)
|
||||||
|
{
|
||||||
|
SWIG_PRV_DATA* t = (SWIG_PRV_DATA*)JSObjectGetPrivate(thisObject);
|
||||||
|
if(t && t->swigCMemOwn) free ((${type}*)t->swigCObject);
|
||||||
|
if(t) free(t);
|
||||||
|
}
|
||||||
|
%}
|
||||||
|
|
||||||
|
But for the above example case of %extend to define a destructor on a struct, we need to override the system to not call
|
||||||
|
free ((${type}*)t->swigCObject);
|
||||||
|
and substitute it with what the user has provided.
|
||||||
|
To solve this, I created a variation fragment called JS_destructoroverridedefn:
|
||||||
|
SWIG_PRV_DATA* t = (SWIG_PRV_DATA*)JSObjectGetPrivate(thisObject);
|
||||||
|
if(t && t->swigCMemOwn) {
|
||||||
|
${type}* arg1 = (${type}*)t->swigCObject;
|
||||||
|
${destructor_action}
|
||||||
|
}
|
||||||
|
if(t) free(t);
|
||||||
|
|
||||||
|
Based on what I saw in the Lua and Ruby modules, I use Getattr(n, "wrap:action")
|
||||||
|
to decide if the user has a preferred destructor action.
|
||||||
|
Based on that, I decide which fragment to use.
|
||||||
|
And in the case of the custom action, I substitute that action in.
|
||||||
|
I noticed that destructor_action has the form
|
||||||
|
delete_MyData(arg1);
|
||||||
|
The explicit arg1 is a little funny, so I structured the fragment to create a temporary variable called arg1 to make the generation easier.
|
||||||
|
This might suggest this solution misunderstands a more complex case.
|
||||||
|
|
||||||
|
Also, there is a problem where destructor_action is always true for me, even when not requesting %extend as above.
|
||||||
|
So this code doesn't actually quite work as I expect. The end result is that the code still works because
|
||||||
|
destructor_action calls free like the original template. The one caveat is the string in destructor_action casts to char* which is wierd.
|
||||||
|
I think there is a deeper underlying SWIG issue because I don't think it should be char*. However, it doesn't really matter for free.
|
||||||
|
|
||||||
|
Maybe the fix for the destructor_action always true problem is that this is supposed to be embedded in the if(Extend) block above.
|
||||||
|
But I don't fully understand the conditions of any of these things, and since it works for the moment, I don't want to break more stuff.
|
||||||
|
*/
|
||||||
|
if(destructor_action) {
|
||||||
|
Template t_dtor = getTemplate("js_dtoroverride");
|
||||||
|
state.clazz(DTOR, wrap_name);
|
||||||
|
t_dtor.replace("${classname_mangled}", state.clazz(NAME_MANGLED))
|
||||||
|
.replace(T_WRAPPER, wrap_name)
|
||||||
|
.replace(T_FREE, free)
|
||||||
|
.replace(T_TYPE, ctype);
|
||||||
|
|
||||||
|
t_dtor.replace("${destructor_action}", destructor_action);
|
||||||
|
Wrapper_pretty_print(t_dtor.str(), f_wrappers);
|
||||||
|
}
|
||||||
|
else {
|
||||||
|
Template t_dtor = getTemplate("js_dtor");
|
||||||
|
state.clazz(DTOR, wrap_name);
|
||||||
|
t_dtor.replace(T_NAME_MANGLED, state.clazz(NAME_MANGLED))
|
||||||
.replace(T_WRAPPER, wrap_name)
|
.replace(T_WRAPPER, wrap_name)
|
||||||
.replace(T_FREE, free)
|
.replace(T_FREE, free)
|
||||||
.replace(T_TYPE, ctype)
|
.replace(T_TYPE, ctype)
|
||||||
.pretty_print(f_wrappers);
|
.pretty_print(f_wrappers);
|
||||||
|
}
|
||||||
|
|
||||||
Delete(p_classtype);
|
Delete(p_classtype);
|
||||||
Delete(ctype);
|
Delete(ctype);
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue