[gjs] jsapi-util-root: Don't destroy GjsMaybeOwned in notify



commit 4b0523ce659e6f2b4cf85c84dcecdcec91d30846
Author: Philip Chimento <philip endlessm com>
Date:   Thu Feb 9 16:52:16 2017 -0800

    jsapi-util-root: Don't destroy GjsMaybeOwned in notify
    
    Previously the code claimed that it was safe to delete a GjsMaybeOwned
    inside its context-destroyed notification callback. However, this is not
    true. Remove the comment saying so, and remove the test that tests it.
    
    We did not actually use this feature in the code.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=776966

 gjs/jsapi-util-root.h     |    7 +------
 test/gjs-test-rooting.cpp |   22 ----------------------
 2 files changed, 1 insertions(+), 28 deletions(-)
---
diff --git a/gjs/jsapi-util-root.h b/gjs/jsapi-util-root.h
index 9cc12ea..6220975 100644
--- a/gjs/jsapi-util-root.h
+++ b/gjs/jsapi-util-root.h
@@ -150,15 +150,10 @@ private:
          * to remove it. */
         m_has_weakref = false;
 
-        /* The object is still live across this callback. Re-entry into the
-         * destructor from the callback should also be safe. */
+        /* The object is still live across this callback. */
         if (m_notify)
             m_notify(handle(), m_data);
 
-        /* If the callback destroyed this wrapper already, we're done. */
-        if (!m_rooted)
-            return;
-
         reset();
     }
 
diff --git a/test/gjs-test-rooting.cpp b/test/gjs-test-rooting.cpp
index 49bdd3d..af68ce0 100644
--- a/test/gjs-test-rooting.cpp
+++ b/test/gjs-test-rooting.cpp
@@ -185,26 +185,6 @@ test_maybe_owned_object_destroyed_after_notify(GjsRootingFixture *fx,
     delete obj;
 }
 
-static void
-delete_obj_callback(JS::HandleObject obj,
-                    void            *data)
-{
-    auto wrapper = static_cast<GjsMaybeOwned<JSObject *> *>(data);
-    delete wrapper;
-}
-
-static void
-test_maybe_owned_can_destroy_object_in_notify_callback(GjsRootingFixture *fx,
-                                                       gconstpointer      unused)
-{
-    auto obj = new GjsMaybeOwned<JSObject *>();
-    obj->root(PARENT(fx)->cx, test_obj_new(fx), delete_obj_callback, obj);
-
-    gjs_unit_test_destroy_context(PARENT(fx));
-    g_assert_true(fx->finalized);
-    /* obj already deleted */
-}
-
 void
 gjs_test_add_tests_for_rooting(void)
 {
@@ -230,8 +210,6 @@ gjs_test_add_tests_for_rooting(void)
                              test_maybe_owned_notify_callback_called_on_context_destroy);
     ADD_CONTEXT_DESTROY_TEST("maybe-owned/object-destroyed-after-notify",
                              test_maybe_owned_object_destroyed_after_notify);
-    ADD_CONTEXT_DESTROY_TEST("maybe-owned/can-destroy-object-in-notify-callback",
-                             test_maybe_owned_can_destroy_object_in_notify_callback);
 
 #undef ADD_CONTEXT_DESTROY_TEST
 }


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]