cppbind: fix possible usage of expired object from Nasal

diff --git a/simgear/nasal/cppbind/Ghost.hxx b/simgear/nasal/cppbind/Ghost.hxx
index b52f825ea0bc2d52e4bc57ab2559becf9b0b43dc..6bde867974dcc33e6c0f5ad67a8c1dcc5e2b31c9 100644 (file)
--- a/simgear/nasal/cppbind/Ghost.hxx
+++ b/simgear/nasal/cppbind/Ghost.hxx
@@ -24,6 +24,8 @@
 #include "NasalObjectHolder.hxx"
 
 #include <simgear/debug/logstream.hxx>
+#include <simgear/structure/SGWeakReferenced.hxx>
+#include <simgear/structure/SGWeakPtr.hxx>
 
 #include <boost/bind.hpp>
 #include <boost/call_traits.hpp>
@@ -121,20 +123,21 @@ namespace nasal
     /**
      * Hold callable method and convert to Nasal function if required.
      */
-    class MethodHolder
+    class MethodHolder:
+      public SGWeakReferenced
     {
       public:
         virtual ~MethodHolder() {}
 
         naRef get_naRef(naContext c)
         {
-          if( !_obj )
-            _obj = ObjectHolder::makeShared(createNasalObject(c));
-          return _obj->get_naRef();
+          if( !_obj.valid() )
+            _obj.reset(createNasalObject(c));
+          return _obj.get_naRef();
         }
 
       protected:
-        ObjectHolderRef _obj;
+        ObjectHolder _obj;
 
         virtual naRef createNasalObject(naContext c) = 0;
     };
@@ -155,6 +158,9 @@ namespace nasal
     {};
   }
 
+  typedef SGSharedPtr<internal::MethodHolder> MethodHolderPtr;
+  typedef SGWeakPtr<internal::MethodHolder> MethodHolderWeakPtr;
+
   /**
    * Class for exposing C++ objects to Nasal
    *
@@ -208,7 +214,6 @@ namespace nasal
       typedef boost::function<naRef(naContext, raw_type&)>          getter_t;
       typedef boost::function<void(naContext, raw_type&, naRef)>    setter_t;
       typedef boost::function<naRef(raw_type&, const CallContext&)> method_t;
-      typedef boost::shared_ptr<internal::MethodHolder> MethodHolderPtr;
 
       class MethodHolder:
         public internal::MethodHolder
@@ -219,11 +224,27 @@ namespace nasal
           {}
 
         protected:
+
+          typedef SGSharedPtr<MethodHolder> SharedPtr;
+          typedef SGWeakPtr<MethodHolder> WeakPtr;
+
           method_t  _method;
 
           virtual naRef createNasalObject(naContext c)
           {
-            return naNewFunc(c, naNewCCodeU(c, &MethodHolder::call, this));
+            return naNewFunc
+            (
+              c,
+              naNewCCodeUD( c,
+                            &MethodHolder::call,
+                            new WeakPtr(this),
+                            &destroyHolder )
+            );
+          }
+
+          static void destroyHolder(void* user_data)
+          {
+            delete static_cast<WeakPtr*>(user_data);
           }
 
           static naRef call( naContext c,
@@ -232,12 +253,16 @@ namespace nasal
                              naRef* args,
                              void* user_data )
           {
-            MethodHolder* holder = static_cast<MethodHolder*>(user_data);
-            if( !holder )
+            WeakPtr* holder_weak = static_cast<WeakPtr*>(user_data);
+            if( !holder_weak )
               naRuntimeError(c, "invalid method holder!");
 
             try
             {
+              SharedPtr holder = holder_weak->lock();
+              if( !holder )
+                throw std::runtime_error("holder has expired");
+
               return holder->_method
               (
                 requireObject(c, me),
@@ -490,7 +515,7 @@ namespace nasal
        */
       Ghost& method(const std::string& name, const method_t& func)
       {
-        _members[name].func.reset( new MethodHolder(func) );
+        _members[name].func = new MethodHolder(func);
         return *this;
       }
 

diff --git a/simgear/nasal/cppbind/NasalObjectHolder.cxx b/simgear/nasal/cppbind/NasalObjectHolder.cxx
index e2f190b0c032cd839ac339090045df6cdc19ccc2..45e929ecfa8c553effc493d6193e084bc9d3a63f 100644 (file)
--- a/simgear/nasal/cppbind/NasalObjectHolder.cxx
+++ b/simgear/nasal/cppbind/NasalObjectHolder.cxx
@@ -25,7 +25,8 @@ namespace nasal
   //----------------------------------------------------------------------------
   ObjectHolder::~ObjectHolder()
   {
-    naGCRelease(_gc_key);
+    if( !naIsNil(_ref) )
+      naGCRelease(_gc_key);
   }
 
   //----------------------------------------------------------------------------
@@ -34,6 +35,32 @@ namespace nasal
     return _ref;
   }
 
+  //----------------------------------------------------------------------------
+  void ObjectHolder::reset()
+  {
+    if( !naIsNil(_ref) )
+      naGCRelease(_gc_key);
+
+    _ref = naNil();
+    _gc_key = 0;
+  }
+
+  //----------------------------------------------------------------------------
+  void ObjectHolder::reset(naRef obj)
+  {
+    if( !naIsNil(_ref) )
+      naGCRelease(_gc_key);
+
+    _ref = obj;
+    _gc_key = naGCSave(obj);
+  }
+
+  //----------------------------------------------------------------------------
+  bool ObjectHolder::valid() const
+  {
+    return !naIsNil(_ref);
+  }
+
   //----------------------------------------------------------------------------
   ObjectHolderRef ObjectHolder::makeShared(naRef obj)
   {
@@ -43,7 +70,16 @@ namespace nasal
   //----------------------------------------------------------------------------
   ObjectHolder::ObjectHolder(naRef obj):
     _ref(obj),
-    _gc_key(naGCSave(obj))
+    _gc_key(0)
+  {
+    if( !naIsNil(obj) )
+      naGCSave(obj);
+  }
+
+  //----------------------------------------------------------------------------
+  ObjectHolder::ObjectHolder():
+    _ref(naNil()),
+    _gc_key(0)
   {
 
   }

diff --git a/simgear/nasal/cppbind/NasalObjectHolder.hxx b/simgear/nasal/cppbind/NasalObjectHolder.hxx
index 47b13d52d216d46a12371270456a3092623c1986..94f94e05c04e059341bc59cc73410f67e64f22b1 100644 (file)
--- a/simgear/nasal/cppbind/NasalObjectHolder.hxx
+++ b/simgear/nasal/cppbind/NasalObjectHolder.hxx
@@ -37,6 +37,16 @@ namespace nasal
   {
     public:
 
+      /**
+       * @param obj Object to save
+       */
+      explicit ObjectHolder(naRef obj);
+
+      /**
+       *
+       */
+      ObjectHolder();
+
       /**
        *
        */
@@ -47,6 +57,23 @@ namespace nasal
        */
       naRef get_naRef() const;
 
+      /**
+       * Release the managed object
+       */
+      void reset();
+
+      /**
+       * Replaces the managed object (the old object is released)
+       *
+       * @param obj New object to save
+       */
+      void reset(naRef obj);
+
+      /**
+       * Check if there is a managed object
+       */
+      bool valid() const;
+
       /**
        * Save the given object as long as the returned holder exists.
        *
@@ -57,11 +84,6 @@ namespace nasal
     protected:
       naRef _ref;
       int _gc_key;
-
-      /**
-       * @param obj Object to save
-       */
-      ObjectHolder(naRef obj);
   };
 
 } // namespace nasal