[gjs] util-root: Require GjsMaybeOwned callback to reset

[Philip Chimento <pchimento src gnome org>]

commit 53e0c86ee955c0a0cca8563786ca3c312b50e1f0
Author: Philip Chimento <philip chimento gmail com>
Date:   Sun Jun 11 12:59:12 2017 -0700

    util-root: Require GjsMaybeOwned callback to reset
    
    In the case of a closure, the GjsMaybeOwned object is embedded as part of
    struct Closure. The context destroy notify callback will invalidate the
    closure, which frees the GjsMaybeOwned object, causing a use-after-free
    when the callback returns and calls reset().
    
    In practice we did not need to call reset() after the callback returns;
    all existing callbacks already call reset(). This patch adds a
    requirement that the callback *must* call reset(), and only calls it
    internally if there was no callback set.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=781799

 gjs/jsapi-util-root.h     |    7 ++++---
 test/gjs-test-rooting.cpp |   15 +++++++++------
 2 files changed, 13 insertions(+), 9 deletions(-)
---
diff --git a/gjs/jsapi-util-root.h b/gjs/jsapi-util-root.h
index 1bdf029..543f537 100644
--- a/gjs/jsapi-util-root.h
+++ b/gjs/jsapi-util-root.h
@@ -170,11 +170,12 @@ private:
          * to remove it. */
         m_has_weakref = false;
 
-        /* The object is still live across this callback. */
+        /* The object is still live entering this callback. The callback
+         * must reset() this wrapper. */
         if (m_notify)
             m_notify(handle(), m_data);
-
-        reset();
+        else
+            reset();
     }
 
 public:
diff --git a/test/gjs-test-rooting.cpp b/test/gjs-test-rooting.cpp
index 240455f..64cf909 100644
--- a/test/gjs-test-rooting.cpp
+++ b/test/gjs-test-rooting.cpp
@@ -13,6 +13,8 @@ struct _GjsRootingFixture {
 
     bool finalized;
     bool notify_called;
+
+    GjsMaybeOwned<JSObject *> *obj;  /* only used in callback test cases */
 };
 
 static void
@@ -220,6 +222,7 @@ context_destroyed(JS::HandleObject obj,
     g_assert_false(fx->notify_called);
     g_assert_false(fx->finalized);
     fx->notify_called = true;
+    fx->obj->reset();
 }
 
 static void
@@ -233,24 +236,24 @@ static void
 test_maybe_owned_notify_callback_called_on_context_destroy(GjsRootingFixture *fx,
                                                            gconstpointer      unused)
 {
-    auto obj = new GjsMaybeOwned<JSObject *>();
-    obj->root(PARENT(fx)->cx, test_obj_new(fx), context_destroyed, fx);
+    fx->obj = new GjsMaybeOwned<JSObject *>();
+    fx->obj->root(PARENT(fx)->cx, test_obj_new(fx), context_destroyed, fx);
 
     gjs_unit_test_destroy_context(PARENT(fx));
     g_assert_true(fx->notify_called);
-    delete obj;
+    delete fx->obj;
 }
 
 static void
 test_maybe_owned_object_destroyed_after_notify(GjsRootingFixture *fx,
                                                gconstpointer      unused)
 {
-    auto obj = new GjsMaybeOwned<JSObject *>();
-    obj->root(PARENT(fx)->cx, test_obj_new(fx), context_destroyed, fx);
+    fx->obj = new GjsMaybeOwned<JSObject *>();
+    fx->obj->root(PARENT(fx)->cx, test_obj_new(fx), context_destroyed, fx);
 
     gjs_unit_test_destroy_context(PARENT(fx));
     g_assert_true(fx->finalized);
-    delete obj;
+    delete fx->obj;
 }
 
 void