changeset 1623:74be0f498b08

Updated mechanism to avoid using deleted objects in RequestAnimationFrame callbacks.
author Benjamin Golinvaux <bgo@osimis.io>
date Wed, 04 Nov 2020 11:39:15 +0100
parents 0f8d6791b403
children 59f95b9ea858
files Applications/Platforms/WebAssembly/WebAssemblyViewport.cpp Applications/Platforms/WebAssembly/WebAssemblyViewport.h Applications/StoneWebViewer/WebAssembly/StoneWebViewer.cpp
diffstat 3 files changed, 36 insertions(+), 42 deletions(-) [+]
line wrap: on
line diff
--- a/Applications/Platforms/WebAssembly/WebAssemblyViewport.cpp	Tue Nov 03 07:04:02 2020 +0100
+++ b/Applications/Platforms/WebAssembly/WebAssemblyViewport.cpp	Wed Nov 04 11:39:15 2020 +0100
@@ -137,10 +137,10 @@
     } 
     else
     {
-      LOG(INFO) << "WebAssemblyViewport::OnRequestAnimationFrame " 
-        << "-- the WebAssemblyViewport is deleted and Paint will " 
-        << "not be called";
+      LOG(TRACE) << "WebAssemblyViewport::OnRequestAnimationFrame: the " << 
+        "WebAssemblyViewport is deleted and Paint will not be called.";
     }
+    WebAssemblyViewport::ReleaseObjectCookie(userData);
     LOG(TRACE) << "Exiting: " << __func__;
     return true;
   }
@@ -220,36 +220,26 @@
     return true;
   }
 
-  void* WebAssemblyViewport::GetObjectCookie()
+  void* WebAssemblyViewport::CreateObjectCookie()
   {
-    if(objectCookie_ != NULL)
-      return objectCookie_;
-    
-    boost::shared_ptr<WebAssemblyViewport>* sharedFromThisPtr = 
-      new boost::shared_ptr<WebAssemblyViewport>();
-    
-    *sharedFromThisPtr = shared_from_this();
-
-    objectCookie_ = reinterpret_cast<void*>(sharedFromThisPtr);
+    boost::weak_ptr<WebAssemblyViewport>* weakThisPtr = 
+      new boost::weak_ptr<WebAssemblyViewport>();
     
-    return objectCookie_;
-  }
+    *weakThisPtr = shared_from_this();
+
+    void* cookie = reinterpret_cast<void*>(weakThisPtr);
 
-  void WebAssemblyViewport::ReleaseObjectCookie(void* cookie)
-  {
-    WebAssemblyViewport* This = DereferenceObjectCookie(cookie);
-    
-    if (This != NULL)
-      This->objectCookie_ = NULL;
+    LOG(TRACE) << "WebAssemblyViewport::CreateObjectCookie() => cookie = " 
+      << cookie << "\n";
 
-    boost::weak_ptr<WebAssemblyViewport>* weakThisPtr = 
-      reinterpret_cast<boost::weak_ptr<WebAssemblyViewport>*>(cookie);
-    
-    delete weakThisPtr;
+    return cookie;
   }
 
   WebAssemblyViewport* WebAssemblyViewport::DereferenceObjectCookie(void* cookie)
   {
+    LOG(TRACE) << "WebAssemblyViewport::DereferenceObjectCookie(cookie = " 
+      << cookie << ")\n";
+
     boost::weak_ptr<WebAssemblyViewport>* weakThisPtr = 
       reinterpret_cast<boost::weak_ptr<WebAssemblyViewport>*>(cookie);
 
@@ -258,11 +248,23 @@
     return sharedThis.get();
   }
 
+  void WebAssemblyViewport::ReleaseObjectCookie(void* cookie)
+  {
+    LOG(TRACE) << "WebAssemblyViewport::ReleaseObjectCookie(cookie = " 
+      << cookie << ")\n";
+
+    boost::weak_ptr<WebAssemblyViewport>* weakThisPtr = 
+      reinterpret_cast<boost::weak_ptr<WebAssemblyViewport>*>(cookie);
+    
+    delete weakThisPtr;
+  }
+
   void WebAssemblyViewport::Invalidate()
   {
+    LOG(TRACE) << "WebAssemblyViewport::Invalidate()\n";
     long id = emscripten_request_animation_frame(OnRequestAnimationFrame, 
-                                                 GetObjectCookie());
-    animationFrameCallbackIds_.push_back(id);
+                                                 CreateObjectCookie());
+    //animationFrameCallbackIds_.push_back(id);
   }
 
   void WebAssemblyViewport::FitForPrint()
@@ -305,8 +307,7 @@
     interactor_(new DefaultViewportInteractor),
     enableEmscriptenMouseEvents_(enableEmscriptenMouseEvents),
     canvasWidth_(0),
-    canvasHeight_(0),
-    objectCookie_(NULL)
+    canvasHeight_(0)
   {
   }
 
@@ -370,13 +371,8 @@
 
   WebAssemblyViewport::~WebAssemblyViewport()
   {
-    // cancel the pending RequestAnimationFrame callbacks
-    for(size_t i = 0; i < animationFrameCallbackIds_.size(); ++i)
-    {
-      long id = animationFrameCallbackIds_[i];
-      emscripten_cancel_animation_frame(id);
-    }
-
+    LOG(TRACE) << "WebAssemblyViewport::~WebAssemblyViewport()\n";
+    
     emscripten_set_resize_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW,
                                    reinterpret_cast<void*>(this),
                                    false,
--- a/Applications/Platforms/WebAssembly/WebAssemblyViewport.h	Tue Nov 03 07:04:02 2020 +0100
+++ b/Applications/Platforms/WebAssembly/WebAssemblyViewport.h	Wed Nov 04 11:39:15 2020 +0100
@@ -61,8 +61,6 @@
     bool                                  enableEmscriptenMouseEvents_;
     unsigned int                          canvasWidth_;
     unsigned int                          canvasHeight_;
-    std::vector<long>                     animationFrameCallbackIds_;
-    void*                                 objectCookie_;
 
     static EM_BOOL OnRequestAnimationFrame(double time, void *userData);
     
@@ -109,12 +107,12 @@
      * This cookie is a resource and must be freed when it is guaranteed 
      * not to be used anymore, with ReleaseObjectCookie
      */
-    void* GetObjectCookie();
+    void* CreateObjectCookie();
 
     /**
      * This static method can be used to dereference a cookie (i.e. retrieve 
      * a pointer to the underlying object) that has been created with 
-     * WebAssemblyViewport::GetObjectCookie()
+     * WebAssemblyViewport::CreateObjectCookie()
      * 
      * If this method returns NULL, it basically means that the 
      * WebAssemblyViewport has already been deleted and that you should NOT 
@@ -128,7 +126,7 @@
      * go wrong.
      * 
      * NEVER call this method on a void* that has not been generated by the
-     * GetObjectCookie method of this class
+     * CreateObjectCookie method of this class
      */
     static WebAssemblyViewport* DereferenceObjectCookie(void* cookie);
 
--- a/Applications/StoneWebViewer/WebAssembly/StoneWebViewer.cpp	Tue Nov 03 07:04:02 2020 +0100
+++ b/Applications/StoneWebViewer/WebAssembly/StoneWebViewer.cpp	Wed Nov 04 11:39:15 2020 +0100
@@ -1793,7 +1793,7 @@
   virtual ~ViewerViewport()
   {
     // Unregister the callbacks to avoid any call with a "void*" that
-    // has been destroyed. "WebAssemblyViewport::GetObjectCookie()"
+    // has been destroyed. "WebAssemblyViewport::CreateObjectCookie()"
     // provides a more advanced alternative.
     emscripten_set_wheel_callback(viewport_->GetCanvasCssSelector().c_str(), this, true, NULL);
     emscripten_set_keydown_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, this, false, NULL);