[gjs/wip/ptomato/778862] WIP - toggle queue manager
- From: Philip Chimento <pchimento src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gjs/wip/ptomato/778862] WIP - toggle queue manager
- Date: Thu, 9 Mar 2017 03:30:51 +0000 (UTC)
commit 8c407b6238ae6577c275806bed1c9d6ee35bf18c
Author: Philip Chimento <philip endlessm com>
Date: Wed Mar 8 19:30:04 2017 -0800
WIP - toggle queue manager
dbus.js | 11 +++
gi/object.cpp | 238 ++++++++++++---------------------------------------------
gi/toggle.h | 123 +++++++++++++++++++++++++++++
gjs-srcs.mk | 1 +
4 files changed, 183 insertions(+), 190 deletions(-)
---
diff --git a/dbus.js b/dbus.js
new file mode 100644
index 0000000..b798d41
--- /dev/null
+++ b/dbus.js
@@ -0,0 +1,11 @@
+const Gio = imports.gi.Gio;
+const Lang = imports.lang;
+
+const App = new Lang.Class({
+ Name: 'App',
+ Extends: Gio.Application,
+ vfunc_activate: function () { this.parent(); },
+ vfunc_dbus_unregister: function () {},
+});
+
+new App({ application_id: 'com.example.Foo' }).run(['foo']);
\ No newline at end of file
diff --git a/gi/object.cpp b/gi/object.cpp
index 3ca662b..eb74368 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -41,6 +41,7 @@
#include "function.h"
#include "proxyutils.h"
#include "param.h"
+#include "toggle.h"
#include "value.h"
#include "closure.h"
#include "gjs_gi_trace.h"
@@ -79,24 +80,6 @@ typedef struct {
GClosure *closure;
} ConnectData;
-typedef enum
-{
- TOGGLE_DOWN,
- TOGGLE_UP,
-} ToggleDirection;
-
-typedef struct
-{
- GObject *gobj;
- ToggleDirection direction;
- guint needs_unref : 1;
-} ToggleRefNotifyOperation;
-
-enum {
- PROP_0,
- PROP_JS_HANDLED,
-};
-
static std::stack<JS::PersistentRootedObject> object_init_list;
static GHashTable *class_init_properties;
@@ -104,7 +87,9 @@ static bool weak_pointer_callback = false;
static std::set<ObjectInstance *> weak_pointer_list;
extern struct JSClass gjs_object_instance_class;
-static volatile gint pending_idle_toggles;
+
+static GSource *idle_queue_drain_source;
+
static std::set<ObjectInstance *> dissociate_list;
GJS_DEFINE_PRIV_FROM_JS(ObjectInstance, gjs_object_instance_class)
@@ -147,26 +132,6 @@ gjs_object_priv_quark (void)
return val;
}
-static GQuark
-gjs_toggle_down_quark (void)
-{
- static GQuark val = 0;
- if (G_UNLIKELY (!val))
- val = g_quark_from_static_string ("gjs::toggle-down-quark");
-
- return val;
-}
-
-static GQuark
-gjs_toggle_up_quark (void)
-{
- static GQuark val = 0;
- if (G_UNLIKELY (!val))
- val = g_quark_from_static_string ("gjs::toggle-up-quark");
-
- return val;
-}
-
/* Plain g_type_query fails and leaves @query uninitialized for
dynamic types.
See https://bugzilla.gnome.org/show_bug.cgi?id=687184 and
@@ -959,64 +924,6 @@ gobj_no_longer_kept_alive_func(JS::HandleObject obj,
weak_pointer_list.erase(priv);
}
-static GQuark
-get_qdata_key_for_toggle_direction(ToggleDirection direction)
-{
- GQuark quark;
-
- switch (direction) {
- case TOGGLE_UP:
- quark = gjs_toggle_up_quark();
- break;
- case TOGGLE_DOWN:
- quark = gjs_toggle_down_quark();
- break;
- default:
- g_assert_not_reached();
- }
-
- return quark;
-}
-
-static bool
-clear_toggle_idle_source(GObject *gobj,
- ToggleDirection direction)
-{
- GQuark qdata_key;
-
- qdata_key = get_qdata_key_for_toggle_direction(direction);
-
- return g_object_steal_qdata(gobj, qdata_key) != NULL;
-}
-
-static bool
-toggle_idle_source_is_queued(GObject *gobj,
- ToggleDirection direction)
-{
- GQuark qdata_key;
-
- qdata_key = get_qdata_key_for_toggle_direction(direction);
-
- return g_object_get_qdata(gobj, qdata_key) != NULL;
-}
-
-static bool
-cancel_toggle_idle(GObject *gobj,
- ToggleDirection direction)
-{
- GQuark qdata_key;
- GSource *source;
-
- qdata_key = get_qdata_key_for_toggle_direction(direction);
-
- source = (GSource*) g_object_steal_qdata(gobj, qdata_key);
-
- if (source)
- g_source_destroy(source);
-
- return source != 0;
-}
-
static void
handle_toggle_down(GObject *gobj)
{
@@ -1066,91 +973,46 @@ handle_toggle_up(GObject *gobj)
}
}
-static gboolean
-idle_handle_toggle(gpointer data)
+static void
+toggle_handler(GObject *gobj,
+ ToggleQueue::Direction direction)
{
- ToggleRefNotifyOperation *operation = (ToggleRefNotifyOperation *) data;
-
- if (!clear_toggle_idle_source(operation->gobj, operation->direction)) {
- /* Already cleared, the JSObject is going away, abort mission */
- goto out;
- }
-
- switch (operation->direction) {
- case TOGGLE_UP:
- handle_toggle_up(operation->gobj);
+ switch (direction) {
+ case ToggleQueue::UP:
+ handle_toggle_up(gobj);
break;
- case TOGGLE_DOWN:
- handle_toggle_down(operation->gobj);
+ case ToggleQueue::DOWN:
+ handle_toggle_down(gobj);
break;
default:
g_assert_not_reached();
}
-
-out:
- return G_SOURCE_REMOVE;
}
-static void
-toggle_ref_notify_operation_free(ToggleRefNotifyOperation *operation)
+static gboolean
+idle_handle_toggle(gpointer unused)
{
- if (operation->needs_unref)
- g_object_unref (operation->gobj);
- g_slice_free(ToggleRefNotifyOperation, operation);
- g_atomic_int_add(&pending_idle_toggles, -1);
+ auto& toggle_queue = ToggleQueue::get_default();
+
+ while (toggle_queue.handle_toggle(toggle_handler))
+ ;
+
+ g_source_destroy(idle_queue_drain_source);
+ idle_queue_drain_source = NULL;
+ return G_SOURCE_REMOVE;
}
static void
-queue_toggle_idle(GObject *gobj,
- ToggleDirection direction)
+ensure_idle_queue_drain(void)
{
- ToggleRefNotifyOperation *operation;
- GQuark qdata_key;
- GSource *source;
-
- operation = g_slice_new0(ToggleRefNotifyOperation);
- operation->direction = direction;
-
- switch (direction) {
- case TOGGLE_UP:
- /* If we're toggling up we take a reference to the object now,
- * so it won't toggle down before we process it. This ensures we
- * only ever have at most two toggle notifications queued.
- * (either only up, or down-up)
- */
- operation->gobj = (GObject*) g_object_ref(gobj);
- operation->needs_unref = true;
- break;
- case TOGGLE_DOWN:
- /* If we're toggling down, we don't need to take a reference since
- * the associated JSObject already has one, and that JSObject won't
- * get finalized until we've completed toggling (since it's rooted,
- * until we unroot it when we dispatch the toggle down idle).
- *
- * Taking a reference now would be bad anyway, since it would force
- * the object to toggle back up again.
- */
- operation->gobj = gobj;
- break;
- default:
- g_assert_not_reached();
- }
-
- qdata_key = get_qdata_key_for_toggle_direction(direction);
+ if (idle_queue_drain_source)
+ return;
- source = g_idle_source_new();
+ GSource *source = g_idle_source_new();
g_source_set_priority(source, G_PRIORITY_HIGH);
- g_source_set_callback(source,
- idle_handle_toggle,
- operation,
- (GDestroyNotify) toggle_ref_notify_operation_free);
-
- g_atomic_int_inc(&pending_idle_toggles);
- g_object_set_qdata (gobj, qdata_key, source);
+ g_source_set_callback(source, idle_handle_toggle, nullptr, nullptr);
g_source_attach (source, NULL);
-
- /* object qdata is piggy-backing off the main loop's ref of the source */
- g_source_unref (source);
+ idle_queue_drain_source = source;
}
static void
@@ -1204,8 +1066,10 @@ wrapped_gobj_toggle_notify(gpointer data,
*/
is_main_thread = _gjs_context_get_is_owner_thread(context);
- toggle_up_queued = toggle_idle_source_is_queued(gobj, TOGGLE_UP);
- toggle_down_queued = toggle_idle_source_is_queued(gobj, TOGGLE_DOWN);
+ auto& toggle_queue = ToggleQueue::get_default();
+
+ toggle_up_queued = toggle_queue.is_queued(gobj, ToggleQueue::UP);
+ toggle_down_queued = toggle_queue.is_queued(gobj, ToggleQueue::DOWN);
if (is_last_ref) {
/* We've transitions from 2 -> 1 references,
@@ -1222,7 +1086,8 @@ wrapped_gobj_toggle_notify(gpointer data,
handle_toggle_down(gobj);
} else {
- queue_toggle_idle(gobj, TOGGLE_DOWN);
+ toggle_queue.enqueue(gobj, ToggleQueue::DOWN);
+ ensure_idle_queue_drain();
}
} else {
/* We've transitioned from 1 -> 2 references.
@@ -1237,7 +1102,8 @@ wrapped_gobj_toggle_notify(gpointer data,
}
handle_toggle_up(gobj);
} else {
- queue_toggle_idle(gobj, TOGGLE_UP);
+ toggle_queue.enqueue(gobj, ToggleQueue::UP);
+ ensure_idle_queue_drain();
}
}
}
@@ -1256,10 +1122,9 @@ release_native_object (ObjectInstance *priv)
void
gjs_object_clear_toggles(void)
{
- while (g_main_context_pending(NULL) &&
- g_atomic_int_get(&pending_idle_toggles) > 0) {
- g_main_context_iteration(NULL, false);
- }
+ auto& toggle_queue = ToggleQueue::get_default();
+ while (toggle_queue.handle_toggle(toggle_handler))
+ ;
}
void
@@ -1384,20 +1249,12 @@ disassociate_js_gobject(GObject *gobj)
g_object_weak_unref(priv->gobj, wrapped_gobj_dispose_notify, priv);
- /* 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
- */
- if (cancel_toggle_idle(gobj, TOGGLE_UP) ||
- cancel_toggle_idle(gobj, TOGGLE_DOWN))
- 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));
+ /* Idles are already checked in the only caller of this
+ function, the toggle ref notify, but let's check again...
+ */
+ auto& toggle_queue = ToggleQueue::get_default();
+ g_assert(!toggle_queue.cancel(gobj, ToggleQueue::UP));
+ g_assert(!toggle_queue.cancel(gobj, ToggleQueue::DOWN));
invalidate_all_signals(priv);
release_native_object(priv);
@@ -1590,8 +1447,9 @@ object_instance_finalize(JSFreeOp *fop,
priv->info ? g_base_info_get_name((GIBaseInfo*) priv->info) : g_type_name(priv->gtype));
}
- had_toggle_up = cancel_toggle_idle(priv->gobj, TOGGLE_UP);
- had_toggle_down = cancel_toggle_idle(priv->gobj, TOGGLE_DOWN);
+ auto& toggle_queue = ToggleQueue::get_default();
+ had_toggle_up = toggle_queue.cancel(priv->gobj, ToggleQueue::UP);
+ had_toggle_down = toggle_queue.cancel(priv->gobj, ToggleQueue::DOWN);
if (!had_toggle_up && had_toggle_down) {
g_error("Finalizing proxy for an object that's scheduled to be unrooted: %s.%s\n",
diff --git a/gi/toggle.h b/gi/toggle.h
new file mode 100644
index 0000000..f22a70b
--- /dev/null
+++ b/gi/toggle.h
@@ -0,0 +1,123 @@
+#include <algorithm>
+#include <deque>
+#include <mutex>
+#include <glib-object.h>
+
+typedef void (*ToggleOperation)(GObject *);
+
+class ToggleQueue {
+public:
+ enum Direction {
+ DOWN,
+ UP
+ };
+
+ typedef void (*Handler)(GObject *, Direction);
+
+private:
+ class Item {
+ unsigned m_needs_unref : 1;
+
+ public:
+ GObject *gobj;
+ Direction direction;
+
+ Item(GObject *a_gobj,
+ Direction a_direction) :
+ gobj(a_gobj),
+ direction(a_direction)
+ {
+ /* If we're toggling up we take a reference to the object now,
+ * so it won't toggle down before we process it. This ensures we
+ * only ever have at most two toggle notifications queued.
+ * (either only up, or down-up)
+ */
+ if (direction == UP) {
+ g_object_ref(gobj);
+ m_needs_unref = true;
+ }
+ /* If we're toggling down, we don't need to take a reference since
+ * the associated JSObject already has one, and that JSObject won't
+ * get finalized until we've completed toggling (since it's rooted,
+ * until we unroot it when we dispatch the toggle down idle).
+ *
+ * Taking a reference now would be bad anyway, since it would force
+ * the object to toggle back up again.
+ */
+ }
+
+ /* If we're cancelling the toggle operation, we don't want to unref
+ * the object after all. */
+ void
+ cancel_unref(void) {
+ m_needs_unref = false;
+ }
+
+ ~Item() {
+ if (m_needs_unref)
+ g_object_unref(gobj);
+ }
+ };
+
+ std::mutex lock;
+ std::deque<Item> q;
+
+ std::deque<Item>::iterator
+ find_operation_locked(GObject *gobj,
+ Direction direction)
+ {
+ return std::find_if(q.begin(), q.end(),
+ [gobj, direction](const Item& item)->bool {
+ return item.gobj == gobj && item.direction == direction;
+ });
+ }
+
+public:
+ bool
+ is_queued(GObject *gobj,
+ Direction direction)
+ {
+ std::lock_guard<std::mutex> hold(lock);
+ return find_operation_locked(gobj, direction) != q.end();
+ }
+
+ bool
+ cancel(GObject *gobj,
+ Direction direction)
+ {
+ std::lock_guard<std::mutex> hold(lock);
+ auto pos = find_operation_locked(gobj, direction);
+
+ if (pos == q.cend())
+ return false;
+
+ pos->cancel_unref();
+ q.erase(pos);
+ return true;
+ }
+
+ bool
+ handle_toggle(Handler handler) {
+ std::lock_guard<std::mutex> hold(lock);
+ if (q.empty())
+ return false;
+
+ handler(q.front().gobj, q.front().direction);
+ q.pop_front();
+ return true;
+ }
+
+ void
+ enqueue(GObject *gobj,
+ Direction direction)
+ {
+ std::lock_guard<std::mutex> hold(lock);
+ q.emplace_back(gobj, direction);
+ }
+
+ static ToggleQueue&
+ get_default(void) {
+ static ToggleQueue the_singleton;
+ return the_singleton;
+ }
+};
diff --git a/gjs-srcs.mk b/gjs-srcs.mk
index 2295bf8..f835570 100644
--- a/gjs-srcs.mk
+++ b/gjs-srcs.mk
@@ -42,6 +42,7 @@ gjs_srcs = \
gi/proxyutils.h \
gi/repo.cpp \
gi/repo.h \
+ gi/toggle.h \
gi/union.cpp \
gi/union.h \
gi/value.cpp \
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
