From e36631d2cb9a3041abc238561a867385329692df Mon Sep 17 00:00:00 2001 From: Lars Kanis <lars@greiz-reinsdorf.de> Date: Sat, 28 Sep 2013 22:50:34 +0200 Subject: [PATCH] Manage GC'ing of FXTreeItem the same way as for FXFolderItem. This fixes issue #10. Stacktrace was: ==4561== Invalid read of size 8 ==4561== at 0x4F726FF: st_lookup (st.c:414) ==4561== by 0x4FCEAD6: rb_method_entry_get_without_cache (vm_method.c:182) ==4561== by 0x4FD94E5: rb_funcall (vm_eval.c:315) ==4561== by 0x810DAF2: FXRbCallVoidMethod(FX::FXObject*, unsigned long) (in /home/lars/.rvm/gems/ruby-2.0.0-p247/gems/fxruby-1.6.28/lib/fox16_c.so) ==4561== by 0x81AC98E: FXRbTreeList::recalc() (in /home/lars/.rvm/gems/ruby-2.0.0-p247/gems/fxruby-1.6.28/lib/fox16_c.so) ==4561== by 0x8FC78DC: FX::FXTreeList::removeItems(FX::FXTreeItem*, FX::FXTreeItem*, unsigned char) (FXTreeList.cpp:2340) ==4561== by 0x8FC7916: FX::FXTreeList::removeItem(FX::FXTreeItem*, unsigned char) (FXTreeList.cpp:2347) ==4561== by 0x829ED0F: FXRbTreeItem::freefunc(FX::FXTreeItem*) (in /home/lars/.rvm/gems/ruby-2.0.0-p247/gems/fxruby-1.6.28/lib/fox16_c.so) ==4561== by 0x4EBA505: finalize_list (gc.c:1415) ==4561== by 0x4EC15A3: rb_gc_call_finalizer_at_exit (gc.c:1552) ==4561== by 0x4EA8064: ruby_cleanup (eval.c:127) ==4561== by 0x4EA8292: ruby_run_node (eval.c:307) ==4561== Address 0x74fb894853ff8550 is not stack'd, malloc'd or (recently) free'd ==4561== segfault_on_close.rb: [BUG] Segmentation fault --- ext/fox16_c/include/FXRbTreeList.h | 6 ++-- ext/fox16_c/markfuncs.cpp | 25 ++----------- ext/fox16_c/unregisterOwnedObjects.cpp | 3 +- swig-interfaces/FXFoldingList.i | 12 +++---- swig-interfaces/FXTreeList.i | 50 ++++++++++---------------- 5 files changed, 31 insertions(+), 65 deletions(-) diff --git a/ext/fox16_c/include/FXRbTreeList.h b/ext/fox16_c/include/FXRbTreeList.h index 9d12e1a..608ebcc 100644 --- a/ext/fox16_c/include/FXRbTreeList.h +++ b/ext/fox16_c/include/FXRbTreeList.h @@ -129,11 +129,11 @@ protected: #include "FXRbObjectVirtuals.h" #include "FXRbTreeItemVirtuals.h" public: - // Pointer to this tree item's owner (if any) - FXWindow *owner; + // Is this tree item owned by an FXTreeList yet? + FXbool owned; public: // Constructor - FXRbTreeItem(const FXString& text,FXIcon* oi=NULL,FXIcon* ci=NULL,void* ptr=NULL) : FXTreeItem(text,oi,ci,ptr),owner(0){} + FXRbTreeItem(const FXString& text,FXIcon* oi=NULL,FXIcon* ci=NULL,void* ptr=NULL) : FXTreeItem(text,oi,ci,ptr),owned(FALSE){} // Mark dependencies for the GC static void markfunc(FXTreeItem* self); diff --git a/ext/fox16_c/markfuncs.cpp b/ext/fox16_c/markfuncs.cpp index 040ea3b..e001bbb 100644 --- a/ext/fox16_c/markfuncs.cpp +++ b/ext/fox16_c/markfuncs.cpp @@ -18,7 +18,7 @@ static void delete_if_not_owned(BASECLASS* self,SUBCLASS*){ if(!dynamic_cast<SUBCLASS*>(self)->owned){ delete self; // also unregisters it } - } + } } FXRbUnregisterRubyObj(self); } @@ -33,7 +33,7 @@ static void delete_if_not_owned_by_app(BASECLASS* self,SUBCLASS*){ if(!dynamic_cast<SUBCLASS*>(self)->ownedByApp){ delete self; // also unregisters it } - } + } } FXRbUnregisterRubyObj(self); } @@ -419,26 +419,7 @@ void FXRbTreeItem::markfunc(FXTreeItem* self){ void FXRbTreeItem::freefunc(FXTreeItem* self){ - if(self!=0){ - if(!FXRbIsBorrowed(self)){ - if(self->isMemberOf(FXMETACLASS(FXRbTreeItem))){ - FXRbTreeItem* treeItem=dynamic_cast<FXRbTreeItem*>(self); - FXASSERT(treeItem); - FXTreeList* treeList=dynamic_cast<FXTreeList*>(treeItem->owner); - if(treeList){ - FXRbUnregisterRubyObj(self); // MAYBE - treeList->removeItem(self,TRUE); - } - else{ - FXTreeListBox* treeListBox=dynamic_cast<FXTreeListBox*>(treeItem->owner); - if(treeListBox){ - treeListBox->removeItem(self); - } - } - } - } - FXRbUnregisterRubyObj(self); - } + delete_if_not_owned(self,reinterpret_cast<FXRbTreeItem*>(0)); } diff --git a/ext/fox16_c/unregisterOwnedObjects.cpp b/ext/fox16_c/unregisterOwnedObjects.cpp index 5cad30f..157025d 100644 --- a/ext/fox16_c/unregisterOwnedObjects.cpp +++ b/ext/fox16_c/unregisterOwnedObjects.cpp @@ -96,7 +96,7 @@ void FXRbTable::unregisterOwnedObjects(FXTable *self) FXRbUnregisterRubyObj(self->getColumnHeader()); for(r=0; r<self->getNumRows(); r++){ for(c=0; c<self->getNumColumns(); c++){ - FXRbUnregisterRubyObj(self->getItem(r,c)); + FXRbUnregisterRubyObj(self->getItem(r,c)); } } } @@ -117,4 +117,3 @@ void FXRbTreeList::unregisterOwnedObjects(FXTreeList *self) // Now zero-out pointers held by still-alive Ruby objects for (FXint i = 0; i < items.no(); i++) FXRbUnregisterRubyObj(items[i]); } - diff --git a/swig-interfaces/FXFoldingList.i b/swig-interfaces/FXFoldingList.i index 22892f8..cbefbd4 100644 --- a/swig-interfaces/FXFoldingList.i +++ b/swig-interfaces/FXFoldingList.i @@ -233,11 +233,11 @@ public: if(FXMALLOC(&strings,FXchar*,len+1)){ for(long i=0;i<len;i++){ VALUE s=rb_ary_entry(stringArray,i); - strings[i]=StringValuePtr(s); + strings[i]=StringValuePtr(s); } - strings[len]=0; + strings[len]=0; self->setHeaders(strings,size); - FXFREE(&strings); + FXFREE(&strings); } } } @@ -296,7 +296,7 @@ public: FXFoldingItem* insertItem(FXFoldingItem* other,FXFoldingItem* father,FXFoldingItem* item,FXbool notify=FALSE){ if(item->isMemberOf(FXMETACLASS(FXRbFoldingItem))){ dynamic_cast<FXRbFoldingItem*>(item)->owned=TRUE; - } + } return self->insertItem(other,father,item,notify); } } @@ -309,7 +309,7 @@ public: FXFoldingItem* appendItem(FXFoldingItem* father,FXFoldingItem* item,FXbool notify=FALSE){ if(item->isMemberOf(FXMETACLASS(FXRbFoldingItem))){ dynamic_cast<FXRbFoldingItem*>(item)->owned=TRUE; - } + } return self->appendItem(father,item,notify); } } @@ -322,7 +322,7 @@ public: FXFoldingItem* prependItem(FXFoldingItem* father,FXFoldingItem* item,FXbool notify=FALSE){ if(item->isMemberOf(FXMETACLASS(FXRbFoldingItem))){ dynamic_cast<FXRbFoldingItem*>(item)->owned=TRUE; - } + } return self->prependItem(father,item,notify); } } diff --git a/swig-interfaces/FXTreeList.i b/swig-interfaces/FXTreeList.i index b11e2dc..43bc2dc 100644 --- a/swig-interfaces/FXTreeList.i +++ b/swig-interfaces/FXTreeList.i @@ -260,55 +260,41 @@ public: /// Insert [possibly subclassed] item under father before other item FXTreeItem* insertItem(FXTreeItem* other,FXTreeItem* father,FXTreeItem* item,FXbool notify=FALSE){ if(item->isMemberOf(FXMETACLASS(FXRbTreeItem))){ - dynamic_cast<FXRbTreeItem*>(item)->owner=self; - } + dynamic_cast<FXRbTreeItem*>(item)->owned=TRUE; + } return self->insertItem(other,father,item,notify); } + } - /// Insert item with given text and optional icons, and user-data pointer under father before other item - FXTreeItem* insertItem(FXTreeItem* other,FXTreeItem* father,const FXString& text,FXIcon* oi=NULL,FXIcon* ci=NULL,void* ITEMDATA=NULL,FXbool notify=FALSE){ - FXTreeItem* item=self->insertItem(other,father,text,oi,ci,ITEMDATA,notify); - if(item->isMemberOf(FXMETACLASS(FXRbTreeItem))){ - dynamic_cast<FXRbTreeItem*>(item)->owner=self; - } - return item; - } + /// Insert item with given text and optional icons, and user-data pointer under father before other item + FXTreeItem* insertItem(FXTreeItem* other,FXTreeItem* father,const FXString& text,FXIcon* oi=NULL,FXIcon* ci=NULL,void* ITEMDATA=NULL,FXbool notify=FALSE); + %extend { /// Append [possibly subclassed] item as last child of father FXTreeItem* appendItem(FXTreeItem* father,FXTreeItem* item,FXbool notify=FALSE){ if(item->isMemberOf(FXMETACLASS(FXRbTreeItem))){ - dynamic_cast<FXRbTreeItem*>(item)->owner=self; - } + dynamic_cast<FXRbTreeItem*>(item)->owned=TRUE; + } return self->appendItem(father,item,notify); } + } - /// Append item with given text and optional icons, and user-data pointer as last child of father - FXTreeItem* appendItem(FXTreeItem* father,const FXString& text,FXIcon* oi=NULL,FXIcon* ci=NULL,void* ITEMDATA=NULL,FXbool notify=FALSE){ - FXTreeItem* item=self->appendItem(father,text,oi,ci,ITEMDATA,notify); - if(item->isMemberOf(FXMETACLASS(FXRbTreeItem))){ - dynamic_cast<FXRbTreeItem*>(item)->owner=self; - } - return item; - } + /// Append item with given text and optional icons, and user-data pointer as last child of father + FXTreeItem* appendItem(FXTreeItem* father,const FXString& text,FXIcon* oi=NULL,FXIcon* ci=NULL,void* ITEMDATA=NULL,FXbool notify=FALSE); + %extend { /// Prepend [possibly subclassed] item as first child of father FXTreeItem* prependItem(FXTreeItem* father,FXTreeItem* item,FXbool notify=FALSE){ if(item->isMemberOf(FXMETACLASS(FXRbTreeItem))){ - dynamic_cast<FXRbTreeItem*>(item)->owner=self; - } + dynamic_cast<FXRbTreeItem*>(item)->owned=TRUE; + } return self->prependItem(father,item,notify); } - - /// Prepend item with given text and optional icons, and user-data pointer as first child of father - FXTreeItem* prependItem(FXTreeItem* father,const FXString& text,FXIcon* oi=NULL,FXIcon* ci=NULL,void* ITEMDATA=NULL,FXbool notify=FALSE){ - FXTreeItem* item=self->prependItem(father,text,oi,ci,ITEMDATA,notify); - if(item->isMemberOf(FXMETACLASS(FXRbTreeItem))){ - dynamic_cast<FXRbTreeItem*>(item)->owner=self; - } - return item; - } } + /// Prepend item with given text and optional icons, and user-data pointer as first child of father + FXTreeItem* prependItem(FXTreeItem* father,const FXString& text,FXIcon* oi=NULL,FXIcon* ci=NULL,void* ITEMDATA=NULL,FXbool notify=FALSE); + /// Move item under father before other item FXTreeItem *moveItem(FXTreeItem* other,FXTreeItem* father,FXTreeItem* item); @@ -360,7 +346,7 @@ public: FXRbUnregisterRubyObj(items[i]); } } - } + } /// Return item width FXint getItemWidth(const FXTreeItem* item) const; -- GitLab