[gjs] Fix assertions during context dispose

[Colin Walters <walters src gnome org>]

commit be5c12c4dc156712a296044916a4108011fe7b3c
Author: Colin Walters <walters verbum org>
Date:   Mon Feb 10 07:21:58 2014 -0500

    Fix assertions during context dispose
    
    The current toggle ref code has and assertion that we don't have a
    DOWN notification pending while the object is supposed to be live -
    and during normal operation, this is true.
    
    Except at context shutdown, we force the JS object to be destroyed,
    which can cause recursion into a dispose handler of a gobject, which
    can then queue a toggle down.  Then we later force dispose that
    object, and find the DOWN pending.
    
    Now, we could simply avoid all of this insanity by default and allow
    the kernel to clean up our state much faster, in a more power
    efficient way by simply not doing any of this...
    
    However, there are valid use cases for wanting "clean" exits, such as
    valgrind.  So this patch fixes things up for the dispose by iterating
    over the entire live object list before we drop into the JS context
    destruction, breaking the JS <-> C association.  This safely clears
    the toggle refs, so the JS side is just JS cleanup, we don't reenter
    GObject land.
    
    This will make several of the OSTree tests written using gjs start
    passing in Continuous again.  \o/
    
    https://bugzilla.gnome.org/show_bug.cgi?id=724060

 Makefile.am           |    1 +
 gi/keep-alive.cpp     |   52 +++++++++++++++++++++++++++++++++++++++++++++-
 gi/keep-alive.h       |   15 +++++++++++++
 gi/object.cpp         |   55 +++++++++++++++++++++++++++++++++++++++++-------
 gi/object.h           |    2 +-
 gjs/context-private.h |   35 +++++++++++++++++++++++++++++++
 gjs/context.cpp       |   19 +++++++++++++++-
 7 files changed, 167 insertions(+), 12 deletions(-)
---
diff --git a/Makefile.am b/Makefile.am
index 321f320..7a56bde 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -56,6 +56,7 @@ nobase_gjs_module_include_HEADERS =   \
 
 noinst_HEADERS +=              \
        gjs/jsapi-private.h     \
+       gjs/context-private.h   \
        gi/proxyutils.h         \
        util/crash.h            \
        util/hash-x32.h         \
diff --git a/gi/keep-alive.cpp b/gi/keep-alive.cpp
index 6c2172d..6c286df 100644
--- a/gi/keep-alive.cpp
+++ b/gi/keep-alive.cpp
@@ -332,7 +332,7 @@ gjs_keep_alive_create(JSContext *context)
 }
 
 JSObject*
-gjs_keep_alive_get_global(JSContext *context)
+gjs_keep_alive_get_global_if_exists (JSContext *context)
 {
     jsval keep_alive;
 
@@ -340,6 +340,17 @@ gjs_keep_alive_get_global(JSContext *context)
 
     if (G_LIKELY (JSVAL_IS_OBJECT(keep_alive)))
         return JSVAL_TO_OBJECT(keep_alive);
+    
+    return NULL;
+}
+
+JSObject*
+gjs_keep_alive_get_global(JSContext *context)
+{
+    JSObject *keep_alive = gjs_keep_alive_get_global_if_exists(context);
+
+    if (G_LIKELY(keep_alive))
+        return keep_alive;
 
     return gjs_keep_alive_create(context);
 }
@@ -381,3 +392,42 @@ gjs_keep_alive_remove_global_child(JSContext         *context,
 
     JS_EndRequest(context);
 }
+
+typedef struct {
+    GHashTableIter hashiter;
+} GjsRealKeepAliveIter;
+
+void
+gjs_keep_alive_iterator_init (GjsKeepAliveIter *iter,
+                              JSObject         *keep_alive)
+{
+    GjsRealKeepAliveIter *real = (GjsRealKeepAliveIter*)iter;
+    KeepAlive *priv = (KeepAlive *) JS_GetPrivate(keep_alive);
+    g_assert(priv != NULL);
+    g_hash_table_iter_init(&real->hashiter, priv->children);
+}
+
+gboolean
+gjs_keep_alive_iterator_next (GjsKeepAliveIter  *iter,
+                              GjsUnrootedFunc    notify_func,
+                              JSObject         **out_child,
+                              void             **out_data)
+{
+    GjsRealKeepAliveIter *real = (GjsRealKeepAliveIter*)iter;
+    gpointer k, v;
+    gboolean ret = FALSE;
+
+    while (g_hash_table_iter_next(&real->hashiter, &k, &v)) {
+        Child *child = (Child*)k;
+
+        if (child->notify != notify_func)
+            continue;
+
+        ret = TRUE;
+        *out_child = child->child;
+        *out_data = child->data;
+        break;
+    }
+
+    return ret;
+}
diff --git a/gi/keep-alive.h b/gi/keep-alive.h
index 4460472..478cd2e 100644
--- a/gi/keep-alive.h
+++ b/gi/keep-alive.h
@@ -63,6 +63,7 @@ void      gjs_keep_alive_remove_child              (JSObject          *keep_aliv
                                                     JSObject          *child,
                                                     void              *data);
 JSObject* gjs_keep_alive_get_global                (JSContext         *context);
+JSObject* gjs_keep_alive_get_global_if_exists      (JSContext         *context);
 void      gjs_keep_alive_add_global_child          (JSContext         *context,
                                                     GjsUnrootedFunc  notify,
                                                     JSObject          *child,
@@ -72,6 +73,20 @@ void      gjs_keep_alive_remove_global_child       (JSContext         *context,
                                                     JSObject          *child,
                                                     void              *data);
 
+typedef struct GjsKeepAliveIter GjsKeepAliveIter;
+struct GjsKeepAliveIter {
+    gpointer dummy[4];
+    guint v;
+    GHashTableIter dummyhiter;
+};
+
+void gjs_keep_alive_iterator_init (GjsKeepAliveIter *iter, JSObject *keep_alive);
+
+gboolean gjs_keep_alive_iterator_next (GjsKeepAliveIter  *iter,
+                                       GjsUnrootedFunc    notify_func,
+                                       JSObject         **out_child,
+                                       void             **out_data);
+
 G_END_DECLS
 
 #endif  /* __GJS_KEEP_ALIVE_H__ */
diff --git a/gi/object.cpp b/gi/object.cpp
index c92b169..e9f59b3 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -42,6 +42,7 @@
 #include <gjs/gjs-module.h>
 #include <gjs/compat.h>
 #include <gjs/type-module.h>
+#include <gjs/context-private.h>
 
 #include <util/log.h>
 #include <util/hash-x32.h>
@@ -1010,6 +1011,15 @@ wrapped_gobj_toggle_notify(gpointer      data,
 {
     gboolean gc_blocked = FALSE;
     gboolean toggle_up_queued, toggle_down_queued;
+    GjsContext *context;
+
+    context = gjs_context_get_current();
+    if (_gjs_context_destroying(context)) {
+        /* Do nothing here - we're in the process of disassociating
+         * the objects.
+         */
+        return;
+    }
 
     /* We only want to touch javascript from one thread.
      * If we're not in that thread, then we need to defer processing
@@ -1065,15 +1075,46 @@ wrapped_gobj_toggle_notify(gpointer      data,
         gjs_unblock_gc();
 }
 
+static void
+release_native_object (ObjectInstance *priv)
+{
+    set_js_obj(priv->gobj, NULL);
+    g_object_remove_toggle_ref(priv->gobj, wrapped_gobj_toggle_notify, NULL);
+    priv->gobj = NULL;
+}
+
 /* At shutdown, we need to ensure we've cleared the context of any
  * pending toggle references.
  */
 void
-gjs_object_process_pending_toggles (void)
+gjs_object_prepare_shutdown (JSContext *context)
 {
-    while (g_main_context_pending (NULL) &&
-           g_atomic_int_get (&pending_idle_toggles) > 0) {
-        g_main_context_iteration (NULL, FALSE);
+    JSObject *keep_alive = gjs_keep_alive_get_global_if_exists (context);
+    GjsKeepAliveIter kiter;
+    JSObject *child;
+    void *data;
+
+    if (!keep_alive)
+        return;
+
+    /* First, get rid of anything left over on the main context */
+    while (g_main_context_pending(NULL) &&
+           g_atomic_int_get(&pending_idle_toggles) > 0) {
+        g_main_context_iteration(NULL, FALSE);
+    }
+
+    /* Now, we iterate over all of the objects, breaking the JS <-> C
+     * associaton.  We avoid the potential recursion implied in:
+     *   toggle ref removal -> gobj dispose -> toggle ref notify
+     * by simply ignoring toggle ref notifications during this process.
+     */
+    gjs_keep_alive_iterator_init(&kiter, keep_alive);
+    while (gjs_keep_alive_iterator_next(&kiter,
+                                        gobj_no_longer_kept_alive_func,
+                                        &child, &data)) {
+        ObjectInstance *priv = (ObjectInstance*)data;
+
+        release_native_object(priv);
     }
 }
 
@@ -1329,10 +1370,8 @@ object_instance_finalize(JSFreeOp  *fop,
                     priv->info ? g_base_info_get_namespace((GIBaseInfo*) priv->info) : "",
                     priv->info ? g_base_info_get_name((GIBaseInfo*) priv->info) : g_type_name(priv->gtype));
         }
-
-        set_js_obj(priv->gobj, NULL);
-        g_object_remove_toggle_ref(priv->gobj, wrapped_gobj_toggle_notify, NULL);
-        priv->gobj = NULL;
+        
+        release_native_object(priv);
     }
 
     if (priv->keep_alive != NULL) {
diff --git a/gi/object.h b/gi/object.h
index 046a71a..7b3b9a4 100644
--- a/gi/object.h
+++ b/gi/object.h
@@ -44,7 +44,7 @@ JSBool    gjs_typecheck_object          (JSContext     *context,
                                          GType          expected_type,
                                          JSBool         throw_error);
 
-void      gjs_object_process_pending_toggles (void);
+void      gjs_object_prepare_shutdown   (JSContext     *context);
 
 G_END_DECLS
 
diff --git a/gjs/context-private.h b/gjs/context-private.h
new file mode 100644
index 0000000..313daf9
--- /dev/null
+++ b/gjs/context-private.h
@@ -0,0 +1,35 @@
+/* -*- mode: C; c-basic-offset: 4; indent-tabs-mode: nil; -*- */
+/*
+ * Copyright (c) 2014 Colin Walters <walters verbum org>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#ifndef __GJS_CONTEXT_PRIVATE_H__
+#define __GJS_CONTEXT_PRIVATE_H__
+
+#include "context.h"
+
+G_BEGIN_DECLS
+
+gboolean     _gjs_context_destroying                  (GjsContext *js_context);
+
+G_END_DECLS
+
+#endif  /* __GJS_CONTEXT_PRIVATE_H__ */
diff --git a/gjs/context.cpp b/gjs/context.cpp
index fbfa9a9..1ea0a3b 100644
--- a/gjs/context.cpp
+++ b/gjs/context.cpp
@@ -25,7 +25,7 @@
 
 #include <gio/gio.h>
 
-#include "context.h"
+#include "context-private.h"
 #include "importer.h"
 #include "jsapi-util.h"
 #include "native.h"
@@ -66,6 +66,8 @@ struct _GjsContext {
 
     char **search_path;
 
+    gboolean destroying;
+
     jsid const_strings[GJS_STRING_LAST];
 };
 
@@ -342,8 +344,15 @@ gjs_context_dispose(GObject *object)
          */
         JS_GC(js_context->runtime);
 
-        gjs_object_process_pending_toggles();
+        js_context->destroying = TRUE;
+
+        /* Now, release all native objects, to avoid recursion between
+         * the JS teardown and the C teardown.  The JSObject proxies
+         * still exist, but point to NULL.
+         */
+        gjs_object_prepare_shutdown(js_context->context);
 
+        /* Tear down JS */
         JS_DestroyContext(js_context->context);
         js_context->context = NULL;
         js_context->runtime = NULL;
@@ -507,6 +516,12 @@ gjs_context_new_with_search_path(char** search_path)
                          NULL);
 }
 
+gboolean
+_gjs_context_destroying (GjsContext *context)
+{
+    return context->destroying;
+}
+
 /**
  * gjs_context_maybe_gc:
  * @context: a #GjsContext