From a69d373e6a113814836e8f9dcd3e848b8bc5247c Mon Sep 17 00:00:00 2001
From: Lars Kanis <kanis@comcard.de>
Date: Tue, 7 Feb 2012 09:58:49 +0100
Subject: [PATCH] Fix Segfault while GC'ing FXWindows

FXWindow destructor calls recalc() and changeFocus() of it's parent windows.
Since these methods are routed back to Ruby code, but calling Ruby code from
GC isn't a good idea, we mark the parent window as "in_gc", so that it will
ignore recalc() and changeFocus() calls completely.

The parent window should also be scheduled to be free'd. In the other case,
the child window would have been marked as used.
---
 ext/fox16/FXRuby.cpp           | 26 ++++++++++++++++++++++++++
 ext/fox16/include/FXRbWindow.h |  4 ++--
 ext/fox16/include/FXRuby.h     |  4 ++++
 ext/fox16/markfuncs.cpp        | 16 ++++++++++++++++
 4 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/ext/fox16/FXRuby.cpp b/ext/fox16/FXRuby.cpp
index 81bb2f1..ad7feb0 100644
--- a/ext/fox16/FXRuby.cpp
+++ b/ext/fox16/FXRuby.cpp
@@ -85,11 +85,15 @@ static st_table * FXRuby_Objects;
  * struct. It identifies the Ruby instance associated with a C++ object.
  * It also indicates whether this is merely a "borrowed" reference to
  * some C++ object (i.e. it's not one we need to destroy later).
+ *
+ * in_gc is set for FXWindows that are in garbage collection and must
+ * not call ruby code anymore.
  */
 
 struct FXRubyObjDesc {
   VALUE obj;
   bool borrowed;
+  bool in_gc;
   };
 
 
@@ -110,6 +114,7 @@ VALUE FXRbNewPointerObj(void *ptr,swig_type_info* ty){
       obj=SWIG_Ruby_NewPointerObj(ptr,ty,1);
       desc->obj=obj;
       desc->borrowed=true;
+      desc->in_gc=false;
       st_insert(FXRuby_Objects,reinterpret_cast<st_data_t>(ptr),reinterpret_cast<st_data_t>(desc));
       return obj;
       }
@@ -140,6 +145,25 @@ bool FXRbIsBorrowed(void* ptr){
     }
   }
 
+bool FXRbSetInGC(const void* ptr, bool enabled){
+  FXASSERT(ptr!=0);
+  FXRubyObjDesc *desc;
+  if(st_lookup(FXRuby_Objects,reinterpret_cast<st_data_t>(ptr),reinterpret_cast<st_data_t *>(&desc))!=0){
+    desc->in_gc=enabled;
+    return enabled;
+    }
+  return false;
+  }
+
+bool FXRbIsInGC(const void* ptr){
+  FXASSERT(ptr!=0);
+  FXRubyObjDesc *desc;
+  if(st_lookup(FXRuby_Objects,reinterpret_cast<st_data_t>(ptr),reinterpret_cast<st_data_t *>(&desc))!=0){
+    return desc->in_gc;
+    }
+  return false;
+  }
+
 
 /**
  * FXRbConvertPtr() is just a wrapper around SWIG_Ruby_ConvertPtr().
@@ -211,6 +235,7 @@ void FXRbRegisterRubyObj(VALUE rubyObj,const void* foxObj) {
   if(FXMALLOC(&desc,FXRubyObjDesc,1)){
     desc->obj=rubyObj;
     desc->borrowed=false;
+    desc->in_gc=false;
     st_insert(FXRuby_Objects,reinterpret_cast<st_data_t>(const_cast<void*>(foxObj)),reinterpret_cast<st_data_t>(desc));
     }
   else{
@@ -1301,6 +1326,7 @@ void FXRbRange2LoHi(VALUE range,FXdouble& lo,FXdouble& hi){
 void FXRbCallVoidMethod(FXObject* recv, ID func) {
   VALUE obj=FXRbGetRubyObj(recv,false);
   FXASSERT(!NIL_P(obj));
+  FXASSERT(!FXRbIsInGC(recv));
   rb_funcall(obj,func,0,NULL);
   }
 
diff --git a/ext/fox16/include/FXRbWindow.h b/ext/fox16/include/FXRbWindow.h
index 78099a9..1e70233 100644
--- a/ext/fox16/include/FXRbWindow.h
+++ b/ext/fox16/include/FXRbWindow.h
@@ -149,7 +149,7 @@ inline void klass ## _dropDisable(klass* self){ \
     FXRbCallVoidMethod(this,rb_intern("killFocus")); \
     } \
   void cls::changeFocus(FXWindow* child){ \
-    FXRbCallVoidMethod(this,rb_intern("changeFocus"),child); \
+    if(!FXRbIsInGC(this)) FXRbCallVoidMethod(this,rb_intern("changeFocus"),child); \
     } \
   void cls::setDefault(FXbool enable){ \
     FXRbCallVoidMethod(this,rb_intern("setDefault"),enable); \
@@ -173,7 +173,7 @@ inline void klass ## _dropDisable(klass* self){ \
     FXRbCallVoidMethod(this,rb_intern("position"),x,y,w,h); \
     } \
   void cls::recalc(){ \
-    FXRbCallVoidMethod(this,rb_intern("recalc")); \
+    if(!FXRbIsInGC(this)) FXRbCallVoidMethod(this,rb_intern("recalc")); \
     } \
   void cls::reparent(FXWindow* father,FXWindow* other){ \
     FXRbCallVoidMethod(this,rb_intern("reparent"),father,other); \
diff --git a/ext/fox16/include/FXRuby.h b/ext/fox16/include/FXRuby.h
index 1a1bd0e..49bb58e 100644
--- a/ext/fox16/include/FXRuby.h
+++ b/ext/fox16/include/FXRuby.h
@@ -73,6 +73,8 @@ VALUE showHelper(VALUE self, int argc, VALUE *argv, TYPE *p, swig_type_info *typ
 // Wrapper around SWIG_Ruby_NewPointerObj()
 VALUE FXRbNewPointerObj(void *ptr, swig_type_info *typeinfo);
 bool FXRbIsBorrowed(void* ptr);
+bool FXRbSetInGC(const void* ptr, bool enabled);
+bool FXRbIsInGC(const void* ptr);
 
 // Wrapper around SWIG_TypeQuery()
 swig_type_info *FXRbTypeQuery(const char *name);
@@ -322,6 +324,7 @@ template<class TYPE>
 void FXRbCallVoidMethod(FXObject* recv,ID func, TYPE& arg){
   VALUE obj=FXRbGetRubyObj(recv,false);
   FXASSERT(!NIL_P(obj));
+  FXASSERT(!FXRbIsInGC(recv));
   rb_funcall(obj,func,1,to_ruby(arg));
   }
 
@@ -336,6 +339,7 @@ template<class TYPE>
 void FXRbCallVoidMethod(const FXObject* recv, ID func, TYPE& arg){
   VALUE obj=FXRbGetRubyObj(recv,false);
   FXASSERT(!NIL_P(obj));
+  FXASSERT(!FXRbIsInGC(recv));
   rb_funcall(obj,func,1,to_ruby(arg));
   }
 
diff --git a/ext/fox16/markfuncs.cpp b/ext/fox16/markfuncs.cpp
index 825a8c7..87b75f6 100644
--- a/ext/fox16/markfuncs.cpp
+++ b/ext/fox16/markfuncs.cpp
@@ -53,6 +53,11 @@ void FXRbObject::markfunc(FXObject* obj){
   FXTRACE((100,"%s::markfunc(%p)\n",obj?obj->getClassName():"FXRbObject",obj));
   }
 
+static void FXRbSetInGCRecursive(FXWindow *window, bool enabled){
+  FXRbSetInGC( window, true );
+  if(window->getParent()) FXRbSetInGCRecursive( window->getParent(), enabled );
+  }
+
 
 void FXRbObject::freefunc(FXObject* self){
   if(self!=0){
@@ -66,6 +71,17 @@ void FXRbObject::freefunc(FXObject* self){
     FXASSERT(classname!=0);
     FXASSERT(strlen(classname)>3);
     if(classname[0]=='F' && classname[1]=='X' && classname[2]=='R' && classname[3]=='b'){
+      // FXWindow destructor calls recalc() and changeFocus() of it's parent window.
+      // Since these methods are routed back to Ruby code, but calling Ruby code from
+      // GC isn't a good idea, we mark the parent window as "in_gc", so that it will
+      // ignore recalc() and changeFocus() calls completely.
+      // The parent window should also be scheduled to be free'd. In the other case,
+      // the child window would have been marked as used.
+      if(self->isMemberOf(FXMETACLASS(FXWindow))){
+        if(FXWindow *parent = dynamic_cast<FXWindow*>(self)->getParent()){
+          FXRbSetInGCRecursive( parent, true );
+          }
+        }
       delete self;
       }
     else{
-- 
GitLab