[pygobject] Fix crashes in various GObject signal handler functions

[Simon Feltman <sfeltman src gnome org>]

commit 80ed803dab3ad914d7214a475e3c6ed743dfdccc
Author: Simon Feltman <sfeltman src gnome org>
Date:   Tue Feb 19 03:07:19 2013 -0800

    Fix crashes in various GObject signal handler functions
    
    Fix crashes in a large amount of signal handler functions exposed
    on the GObject module. This is possible now that the underlying
    GObject pointer is exposed to Python as a PyCapsule which marshaling
    can handle. The following functions in the GObject module have been
    verified:
    
    signal_handler_unblock
    signal_handler_disconnect
    signal_handler_is_connected
    signal_stop_emission
    signal_stop_emission_by_name
    signal_has_handler_pending
    signal_connect_closure
    signal_connect_closure_by_id
    signal_handler_find
    signal_handlers_destroy
    
    https://bugzilla.gnome.org/show_bug.cgi?id=633927

 gi/_gobject/gobjectmodule.c |   57 ++--------------
 gi/overrides/GObject.py     |  158 +++++++++++++++++++++++++++++++++----------
 tests/test_signal.py        |  121 ++++++++++++++++++++++++++++++++-
 3 files changed, 247 insertions(+), 89 deletions(-)
---
diff --git a/gi/_gobject/gobjectmodule.c b/gi/_gobject/gobjectmodule.c
index fda21e7..9fb7564 100644
--- a/gi/_gobject/gobjectmodule.c
+++ b/gi/_gobject/gobjectmodule.c
@@ -37,7 +37,6 @@
 #include "pygpointer.h"
 #include "pygtype.h"
 
-static PyObject *_pyg_signal_accumulator_true_handled_func;
 static GHashTable *log_handlers = NULL;
 static gboolean log_handlers_disabled = FALSE;
 
@@ -340,17 +339,13 @@ create_signal (GType instance_type, const gchar *signal_name, PyObject *tuple)
        Py_DECREF(item);
     }
 
-    if (py_accum == _pyg_signal_accumulator_true_handled_func)
-        accumulator = g_signal_accumulator_true_handled;
-    else {
-        if (py_accum != NULL && py_accum != Py_None) {
-            accum_data = g_new(PyGSignalAccumulatorData, 1);
-            accum_data->callable = py_accum;
-            Py_INCREF(py_accum);
-            accum_data->user_data = py_accum_data;
-            Py_XINCREF(py_accum_data);
-            accumulator = _pyg_signal_accumulator;
-        }
+    if (py_accum != NULL && py_accum != Py_None) {
+        accum_data = g_new(PyGSignalAccumulatorData, 1);
+        accum_data->callable = py_accum;
+        Py_INCREF(py_accum);
+        accum_data->user_data = py_accum_data;
+        Py_XINCREF(py_accum_data);
+        accumulator = _pyg_signal_accumulator;
     }
 
     signal_id = g_signal_newv(signal_name, instance_type, signal_flags,
@@ -1671,38 +1666,6 @@ pyg_add_emission_hook(PyGObject *self, PyObject *args)
 }
 
 static PyObject *
-pyg_remove_emission_hook(PyGObject *self, PyObject *args)
-{
-    PyObject *pygtype, *repr;
-    char *name;
-    guint signal_id;
-    gulong hook_id;
-    GType gtype;
-
-    if (!PyArg_ParseTuple(args, "Osk:gobject.remove_emission_hook",
-                         &pygtype, &name, &hook_id))
-       return NULL;
-
-    if ((gtype = pyg_type_from_object(pygtype)) == 0) {
-       return NULL;
-    }
-
-    if (!g_signal_parse_name(name, gtype, &signal_id, NULL, TRUE)) {
-       repr = PyObject_Repr((PyObject*)self);
-       PyErr_Format(PyExc_TypeError, "%s: unknown signal name: %s",
-                 PYGLIB_PyUnicode_AsString(repr),
-                 name);
-       Py_DECREF(repr);
-       return NULL;
-    }
-
-    g_signal_remove_emission_hook(signal_id, hook_id);
-
-    Py_INCREF(Py_None);
-    return Py_None;
-}
-
-static PyObject *
 pyg__install_metaclass(PyObject *dummy, PyTypeObject *metaclass)
 {
     Py_INCREF(metaclass);
@@ -1733,8 +1696,6 @@ static PyMethodDef _gobject_functions[] = {
       (PyCFunction)pyg_signal_accumulator_true_handled, METH_VARARGS },
     { "add_emission_hook",
       (PyCFunction)pyg_add_emission_hook, METH_VARARGS },
-    { "remove_emission_hook",
-      (PyCFunction)pyg_remove_emission_hook, METH_VARARGS },
     { "_install_metaclass",
       (PyCFunction)pyg__install_metaclass, METH_O },
 
@@ -2233,10 +2194,6 @@ PYGLIB_MODULE_START(_gobject, "_gobject")
     pygobject_enum_register_types(d);
     pygobject_flags_register_types(d);
 
-      /* signal registration recognizes this special accumulator 'constant' */
-    _pyg_signal_accumulator_true_handled_func = \
-        PyDict_GetItemString(d, "signal_accumulator_true_handled");
-
     pygobject_api_functions.threads_enabled = pyglib_threads_enabled();
     _pyglib_notify_on_enabling_threads(pyg_note_threads_enabled);
 }
diff --git a/gi/overrides/GObject.py b/gi/overrides/GObject.py
index 2176d84..883edac 100644
--- a/gi/overrides/GObject.py
+++ b/gi/overrides/GObject.py
@@ -3,7 +3,7 @@
 #
 # Copyright (C) 2012 Canonical Ltd.
 # Author: Martin Pitt <martin pitt ubuntu com>
-# Copyright (C) 2012 Simon Feltman <sfeltman src gnome org>
+# Copyright (C) 2012-2013 Simon Feltman <sfeltman src gnome org>
 # Copyright (C) 2012 Bastian Winkler <buz netbuz org>
 #
 # This library is free software; you can redistribute it and/or
@@ -23,6 +23,7 @@
 
 import sys
 import warnings
+import functools
 from collections import namedtuple
 
 import gi.overrides
@@ -193,20 +194,14 @@ __all__ += ['GBoxed', 'GEnum', 'GFlags', 'GInterface', 'GObject',
             'Warning']
 
 
-add_emission_hook = _gobject.add_emission_hook
 features = _gobject.features
 list_properties = _gobject.list_properties
 new = _gobject.new
 pygobject_version = _gobject.pygobject_version
-remove_emission_hook = _gobject.remove_emission_hook
-signal_accumulator_true_handled = _gobject.signal_accumulator_true_handled
-signal_new = _gobject.signal_new
 threads_init = _gobject.threads_init
 type_register = _gobject.type_register
-__all__ += ['add_emission_hook', 'features', 'list_properties',
-            'new', 'pygobject_version', 'remove_emission_hook',
-            'signal_accumulator_true_handled',
-            'signal_new', 'threads_init', 'type_register']
+__all__ += ['features', 'list_properties', 'new',
+            'pygobject_version', 'threads_init', 'type_register']
 
 
 class Value(GObjectModule.Value):
@@ -417,6 +412,20 @@ def signal_query(id_or_name, type_=None):
 __all__.append('signal_query')
 
 
+def _get_instance_for_signal(obj):
+    if isinstance(obj, GObjectModule.Object):
+        return obj.__gpointer__
+    else:
+        raise TypeError('Unsupported object "%s" for signal function' % obj)
+
+
+def _wrap_signal_func(func):
+    @functools.wraps(func)
+    def wrapper(obj, *args, **kwargs):
+        return func(_get_instance_for_signal(obj), *args, **kwargs)
+    return wrapper
+
+
 class _HandlerBlockManager(object):
     def __init__(self, obj, handler_id):
         self.obj = obj
@@ -426,7 +435,101 @@ class _HandlerBlockManager(object):
         pass
 
     def __exit__(self, exc_type, exc_value, traceback):
-        self.obj.handler_unblock(self.handler_id)
+        signal_handler_unblock(self.obj, self.handler_id)
+
+
+def signal_handler_block(obj, handler_id):
+    """Blocks the signal handler from being invoked until handler_unblock() is called.
+
+    Returns a context manager which optionally can be used to
+    automatically unblock the handler:
+
+    >>> with GObject.signal_handler_block(obj, id):
+    >>>    pass
+    """
+    GObjectModule.signal_handler_block(_get_instance_for_signal(obj), handler_id)
+    return _HandlerBlockManager(obj, handler_id)
+
+__all__.append('signal_handler_block')
+
+
+# The following functions wrap GI functions but coerce the first arg into
+# something compatible with gpointer
+
+signal_handler_unblock = _wrap_signal_func(GObjectModule.signal_handler_unblock)
+signal_handler_disconnect = _wrap_signal_func(GObjectModule.signal_handler_disconnect)
+signal_handler_is_connected = _wrap_signal_func(GObjectModule.signal_handler_is_connected)
+signal_stop_emission = _wrap_signal_func(GObjectModule.signal_stop_emission)
+signal_stop_emission_by_name = _wrap_signal_func(GObjectModule.signal_stop_emission_by_name)
+signal_has_handler_pending = _wrap_signal_func(GObjectModule.signal_has_handler_pending)
+signal_get_invocation_hint = _wrap_signal_func(GObjectModule.signal_get_invocation_hint)
+signal_connect_closure = _wrap_signal_func(GObjectModule.signal_connect_closure)
+signal_connect_closure_by_id = _wrap_signal_func(GObjectModule.signal_connect_closure_by_id)
+signal_handler_find = _wrap_signal_func(GObjectModule.signal_handler_find)
+signal_handlers_destroy = _wrap_signal_func(GObjectModule.signal_handlers_destroy)
+signal_handlers_block_matched = _wrap_signal_func(GObjectModule.signal_handlers_block_matched)
+signal_handlers_unblock_matched = _wrap_signal_func(GObjectModule.signal_handlers_unblock_matched)
+signal_handlers_disconnect_matched = _wrap_signal_func(GObjectModule.signal_handlers_disconnect_matched)
+
+__all__ += ['signal_handler_unblock',
+            'signal_handler_disconnect', 'signal_handler_is_connected',
+            'signal_stop_emission', 'signal_stop_emission_by_name',
+            'signal_has_handler_pending', 'signal_get_invocation_hint',
+            'signal_connect_closure', 'signal_connect_closure_by_id',
+            'signal_handler_find', 'signal_handlers_destroy',
+            'signal_handlers_block_matched', 'signal_handlers_unblock_matched',
+            'signal_handlers_disconnect_matched']
+
+
+def signal_parse_name(detailed_signal, itype, force_detail_quark):
+    """Parse a detailed signal name into (signal_id, detail).
+
+    :Raises ValueError:
+        If the given signal is unknown.
+
+    :Returns:
+        Tuple of (signal_id, detail)
+    """
+    res, signal_id, detail = GObjectModule.signal_parse_name(detailed_signal, itype,
+                                                             force_detail_quark)
+    if res:
+        return signal_id, detail
+    else:
+        raise ValueError('%s: unknown signal name: %s' % (itype, detailed_signal))
+
+__all__.append('signal_parse_name')
+
+
+def remove_emission_hook(obj, detailed_signal, hook_id):
+    signal_id, detail = signal_parse_name(detailed_signal, obj, True)
+    GObjectModule.signal_remove_emission_hook(signal_id, hook_id)
+
+__all__.append('remove_emission_hook')
+
+
+# GObject accumulators with pure Python implementations
+# These return a tuple of (continue_emission, accumulation_result)
+
+def signal_accumulator_first_wins(ihint, return_accu, handler_return, user_data=None):
+    # Stop emission but return the result of the last handler
+    return (False, handler_return)
+
+__all__.append('signal_accumulator_first_wins')
+
+
+def signal_accumulator_true_handled(ihint, return_accu, handler_return, user_data=None):
+    # Stop emission if the last handler returns True
+    return (not handler_return, handler_return)
+
+__all__.append('signal_accumulator_true_handled')
+
+
+# Statically bound signal functions which need to clobber GI (for now)
+
+add_emission_hook = _gobject.add_emission_hook
+signal_new = _gobject.signal_new
+
+__all__ += ['add_emission_hook', 'signal_new']
 
 
 class _FreezeNotifyManager(object):
@@ -500,21 +603,6 @@ class Object(GObjectModule.Object):
     __copy__ = _gobject.GObject.__copy__
     __deepcopy__ = _gobject.GObject.__deepcopy__
 
-    def handler_block(self, handler_id):
-        """Blocks the signal handler from being invoked until handler_unblock() is called.
-
-        Returns a context manager which optionally can be used to
-        automatically unblock the handler:
-
-        >>> with obj.handler_block(id):
-        >>>    pass
-        """
-        GObjectModule.signal_handler_block(self.__gpointer__, handler_id)
-        return _HandlerBlockManager(self, handler_id)
-
-    def handler_unblock(self, handler_id):
-        GObjectModule.signal_handler_unblock(self.__gpointer__, handler_id)
-
     def freeze_notify(self):
         """Freezes the object's property-changed notification queue.
 
@@ -530,27 +618,25 @@ class Object(GObjectModule.Object):
         super(Object, self).freeze_notify()
         return _FreezeNotifyManager(self)
 
-    def handler_disconnect(self, handler_id):
-        GObjectModule.signal_handler_disconnect(self.__gpointer__, handler_id)
-
-    def handler_is_connected(self, handler_id):
-        return bool(GObjectModule.signal_handler_is_connected(self.__gpointer__, handler_id))
-
-    def stop_emission_by_name(self, detailed_signal):
-        GObjectModule.signal_stop_emission_by_name(self.__gpointer__, detailed_signal)
-
     #
     # Aliases
     #
-    disconnect = handler_disconnect
+
+    disconnect = signal_handler_disconnect
+    handler_block = signal_handler_block
+    handler_unblock = signal_handler_unblock
+    handler_disconnect = signal_handler_disconnect
+    handler_is_connected = signal_handler_is_connected
+    stop_emission_by_name = signal_stop_emission_by_name
 
     #
     # Deprecated Methods
     #
+
     def stop_emission(self, detailed_signal):
         """Deprecated, please use stop_emission_by_name."""
         warnings.warn(self.stop_emission.__doc__, PyGIDeprecationWarning, stacklevel=2)
-        return self.stop_emission_by_name(detailed_signal)
+        return signal_stop_emission_by_name(self, detailed_signal)
 
     emit_stop_by_name = stop_emission
 
diff --git a/tests/test_signal.py b/tests/test_signal.py
index afa9926..776ad7a 100644
--- a/tests/test_signal.py
+++ b/tests/test_signal.py
@@ -120,7 +120,9 @@ class Foo(GObject.GObject):
         'my-acc-signal': (GObject.SignalFlags.RUN_LAST, GObject.TYPE_INT,
                           (), my_accumulator, "accum data"),
         'my-other-acc-signal': (GObject.SignalFlags.RUN_LAST, GObject.TYPE_BOOLEAN,
-                                (), GObject.signal_accumulator_true_handled)
+                                (), GObject.signal_accumulator_true_handled),
+        'my-acc-first-wins': (GObject.SignalFlags.RUN_LAST, GObject.TYPE_BOOLEAN,
+                              (), GObject.signal_accumulator_first_wins)
         }
 
 
@@ -148,6 +150,16 @@ class TestAccumulator(unittest.TestCase):
         inst.emit("my-other-acc-signal")
         self.assertEqual(self.__true_val, 2)
 
+    def test_accumulator_first_wins(self):
+        # First signal hit will always win
+        inst = Foo()
+        inst.connect("my-acc-first-wins", self._true_handler3)
+        inst.connect("my-acc-first-wins", self._true_handler1)
+        inst.connect("my-acc-first-wins", self._true_handler2)
+        self.__true_val = None
+        inst.emit("my-acc-first-wins")
+        self.assertEqual(self.__true_val, 3)
+
     def _true_handler1(self, obj):
         self.__true_val = 1
         return False
@@ -257,11 +269,88 @@ class TestEmissionHook(unittest.TestCase):
         self.assertEqual(obj.status, 3)
 
 
+class TestMatching(unittest.TestCase):
+    class Object(GObject.Object):
+        status = 0
+        prop = GObject.Property(type=int, default=0)
+
+        @GObject.Signal()
+        def my_signal(self):
+            pass
+
+    @unittest.expectedFailure  # https://bugzilla.gnome.org/show_bug.cgi?id=692918
+    def test_signal_handler_block_matching(self):
+        def dummy(*args):
+            "Hack to work around: "
+
+        def foo(obj):
+            obj.status += 1
+
+        obj = self.Object()
+        handler_id = GObject.signal_connect_closure(obj, 'my-signal', foo, after=False)
+        handler_id
+
+        self.assertEqual(obj.status, 0)
+        obj.emit('my-signal')
+        self.assertEqual(obj.status, 1)
+
+        # Blocking by match criteria disables the foo callback
+        signal_id, detail = GObject.signal_parse_name('my-signal', obj, True)
+        count = GObject.signal_handlers_block_matched(obj,
+                                                      GObject.SignalMatchType.ID | 
GObject.SignalMatchType.CLOSURE,
+                                                      signal_id=signal_id, detail=detail,
+                                                      closure=foo, func=dummy, data=dummy)
+        self.assertEqual(count, 1)
+        obj.emit('my-signal')
+        self.assertEqual(obj.status, 1)
+
+        # Unblocking by the same match criteria allows callback to work again
+        count = GObject.signal_handlers_unblock_matched(obj,
+                                                        GObject.SignalMatchType.ID | 
GObject.SignalMatchType.CLOSURE,
+                                                        signal_id=signal_id, detail=detail,
+                                                        closure=foo, func=dummy, data=dummy)
+        self.assertEqual(count, 1)
+        obj.emit('my-signal')
+        self.assertEqual(obj.status, 2)
+
+        # Disconnecting by match criteria completely removes the handler
+        count = GObject.signal_handlers_disconnect_matched(obj,
+                                                           GObject.SignalMatchType.ID | 
GObject.SignalMatchType.CLOSURE,
+                                                           signal_id=signal_id, detail=detail,
+                                                           closure=foo, func=dummy, data=dummy)
+        self.assertEqual(count, 1)
+        obj.emit('my-signal')
+        self.assertEqual(obj.status, 2)
+
+    def test_signal_handler_find(self):
+        def dummy(*args):
+            "Hack to work around: "
+
+        def foo(obj):
+            obj.status += 1
+
+        obj = self.Object()
+        handler_id = GObject.signal_connect_closure(obj, 'my-signal', foo, after=False)
+
+        signal_id, detail = GObject.signal_parse_name('my-signal', obj, True)
+        found_id = GObject.signal_handler_find(obj,
+                                               GObject.SignalMatchType.ID,
+                                               signal_id=signal_id, detail=detail,
+                                               closure=None, func=dummy, data=dummy)
+        self.assertEqual(handler_id, found_id)
+
+
 class TestClosures(unittest.TestCase):
     def setUp(self):
         self.count = 0
         self.emission_stopped = False
         self.emission_error = False
+        self.handler_pending = False
+
+    def _callback_handler_pending(self, e):
+        signal_id, detail = GObject.signal_parse_name('signal', e, True)
+        self.handler_pending = GObject.signal_has_handler_pending(e, signal_id, detail,
+                                                                  may_be_blocked=False)
 
     def _callback(self, e):
         self.count += 1
@@ -329,8 +418,8 @@ class TestClosures(unittest.TestCase):
 
     def test_handler_unblock(self):
         e = E()
-        signal_id = e.connect('signal', self._callback)
-        e.handler_block(signal_id)
+        handler_id = e.connect('signal', self._callback)
+        e.handler_block(handler_id)
         e.handler_unblock_by_func(self._callback)
         e.emit('signal')
         self.assertEqual(self.count, 1)
@@ -369,6 +458,32 @@ class TestClosures(unittest.TestCase):
         data = c.emit("my_signal", "\01\00\02")
         self.assertEqual(data, "\02\00\01")
 
+    def test_handler_pending(self):
+        obj = F()
+        obj.connect('signal', self._callback_handler_pending)
+        obj.connect('signal', self._callback)
+
+        self.assertEqual(self.count, 0)
+        self.assertEqual(self.handler_pending, False)
+
+        obj.emit('signal')
+        self.assertEqual(self.count, 1)
+        self.assertEqual(self.handler_pending, True)
+
+    def test_signal_handlers_destroy(self):
+        obj = F()
+        obj.connect('signal', self._callback)
+        obj.connect('signal', self._callback)
+        obj.connect('signal', self._callback)
+
+        obj.emit('signal')
+        self.assertEqual(self.count, 3)
+
+        # count should remain at 3 after all handlers are destroyed
+        GObject.signal_handlers_destroy(obj)
+        obj.emit('signal')
+        self.assertEqual(self.count, 3)
+
 
 class SigPropClass(GObject.GObject):
     __gsignals__ = {'my_signal': (GObject.SignalFlags.RUN_FIRST, None,