[gjs] Revert "WIP - clear toggles before GC"

[Philip Chimento <pchimento src gnome org>]

commit 1954942be6a0faeb49484b79a671588a6f012025
Author: Philip Chimento <philip chimento gmail com>
Date:   Thu Mar 9 00:33:19 2017 -0800

    Revert "WIP - clear toggles before GC"
    
    This reverts commit c840342f39e800bc12b68bd28b749b17321cd61a.
    Pushed by accident.

 dbus.js       |   11 ----------
 gi/object.cpp |   62 ++++++++++++++++++++++++++++++++------------------------
 2 files changed, 35 insertions(+), 38 deletions(-)
---
diff --git a/gi/object.cpp b/gi/object.cpp
index a67d1c3..6d3ef97 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -1007,13 +1007,28 @@ wrapped_gobj_toggle_notify(gpointer      data,
     /* We only want to touch javascript from one thread.
      * If we're not in that thread, then we need to defer processing
      * to it.
-     * Toggling down is simple, because we're creating more garbage. So we
-     * just unroot the object, make it a weak pointer, and wait for the next
-     * GC cycle.
+     * In case we're toggling up (and thus rooting the JS object) we
+     * also need to take care if GC is running. The marking side
+     * of it is taken care by JS::Heap, which we use in KeepAlive,
+     * so we're safe. As for sweeping, it is too late: the JS object
+     * is dead, and attempting to keep it alive would soon crash
+     * the process. Plus, if we touch the JSAPI, libmozjs aborts in
+     * the first BeginRequest.
+     * Thus, in the toggleup+sweeping case we deassociate the object
+     * and the wrapper and let the wrapper die. Then, if the object
+     * appears again, we log a critical.
+     * In practice, a toggle up during JS finalize can only happen
+     * for temporary refs/unrefs of objects that are garbage anyway,
+     * because JS code is never invoked while the finalizers run
+     * and C code needs to clean after itself before it returns
+     * from dispose()/finalize().
+     * On the other hand, toggling down is a lot simpler, because
+     * we're creating more garbage. So we just add the object to
+     * the keep alive and wait for the next GC cycle.
      *
-     * Note that one would think that toggling up (and thus rooting the JS
-     * object) only happens in the main thread (because toggling up is the
-     * result of the JS object, previously visible only to JS code, becoming
+     * Note that one would think that toggling up only happens
+     * in the main thread (because toggling up is the result of
+     * the JS object, previously visible only to JS code, becoming
      * visible to the refcounted C world), but because of weird
      * weak singletons like g_bus_get_sync() objects can see toggle-ups
      * from different threads too.
@@ -1130,20 +1145,6 @@ static void
 update_heap_wrapper_weak_pointers(JSRuntime *rt,
                                   gpointer   data)
 {
-    /* In case we're toggling up we also need to take care if GC is running.
-     * The marking side of it is taken care by JS::Heap, which we use in
-     * GjsMaybeOwned, so we're safe. As for sweeping, it is too late: the JS
-     * object is dead, and attempting to keep it alive would soon crash
-     * the process. Plus, if we touch the JSAPI, libmozjs aborts in
-     * the first BeginRequest.
-     * In practice, a toggle up during JS finalize can only happen
-     * for temporary refs/unrefs of objects that are garbage anyway,
-     * because JS code is never invoked while the finalizers run
-     * and C code needs to clean after itself before it returns
-     * from dispose()/finalize(). */
-    /* FIXME: Shouldn't we do this at the start of GC instead of at the end? */
-    gjs_object_clear_toggles();
-
     for (auto iter = weak_pointer_list.begin(); iter != weak_pointer_list.end(); ) {
         ObjectInstance *priv = *iter;
         if (priv->keep_alive.rooted() || priv->keep_alive == nullptr ||
@@ -1213,15 +1214,22 @@ disassociate_js_gobject(GObject *gobj)
 
     g_object_weak_unref(priv->gobj, wrapped_gobj_dispose_notify, priv);
 
-    /* Idles are already checked in the only caller of this
-       function, the weak pointer callback, but let's check again...
-    */
+    /* FIXME: this check fails when JS code runs after the main loop ends,
+     * because the idle functions are not dispatched without a main loop.
+     * The only situation I'm aware of where this happens is during the
+     * dbus_unregister stage in GApplication. Ideally this check should be an
+     * assertion.
+     * https://bugzilla.gnome.org/show_bug.cgi?id=778862
+     */
     auto& toggle_queue = ToggleQueue::get_default();
     auto had_toggles = toggle_queue.cancel(gobj);
-    g_assert(((void) "Object dissociated while it was queued to toggle down",
-              !std::get<ToggleQueue::DOWN>(had_toggles)));
-    g_assert(((void) "Object dissociated while it was queued to toggle up",
-              !std::get<ToggleQueue::UP>(had_toggles)));
+    if (std::get<ToggleQueue::UP>(had_toggles) ||
+        std::get<ToggleQueue::DOWN>(had_toggles))
+        g_critical("JS object wrapper for GObject %p (%s) is being released "
+                   "while toggle references are still pending. This may happen "
+                   "on exit in Gio.Application.vfunc_dbus_unregister(). If you "
+                   "encounter it another situation, please report a GJS bug.",
+                   gobj, G_OBJECT_TYPE_NAME(gobj));
 
     invalidate_all_signals(priv);
     release_native_object(priv);