[pygobject/pygobject-2-28] Test case with John's fix for crash with C arrays and a GError is set.



commit 6d6d4fcf4678b512558c2c0e44c0c2e235c810f4
Author: Laszlo Pandy <lpandy src gnome org>
Date:   Wed Feb 23 12:05:03 2011 +0100

    Test case with John's fix for crash with C arrays and a GError is set.
    
    I have added a test case, and made a few fixes to John's patch, but the
    solution is the same his.
    
    Workaround a bug when freeing C array types
    
     * This is a hack and there is really no way around it without ripping out
        the current array handling code which spans between pygi-invoke.c and
        pygi-argument.c and completely rewriting it.
      * The is no time before our stable release
      * This patch trades a segfault for a leak in the very unusual case where
        an error occures inside an interface that takes one or more C arrays. Since
        we wrap C arrays in GArrays internally but have to unwrap them to send them
        to the introspected C function, there is a period of time where an error
        can occure with the C array in an unknown state (some being true C arrays
        and others still wrapped in a GArray)
      * This patch adds a c_arrays_are_wrapped state to signal that it is safe to
        free them.  However since c_arrays_are_wrapped can only track arrays
        as a group, not individually, if it is set to FALSE we can not assume
        that every array is a pure C array, so instead we will simply leak them
        to avoid incorrectly freeing one and causing a segfault.
      * This issue is fixed in the invoke rewrite branch as it treats C arrays and
        GArrays separately, however that branch is not yet ready to be merged and
        won't be until the next release.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=642708

 gi/pygi-invoke.c |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++----
 tests/test_gi.py |    9 +++++++++
 2 files changed, 59 insertions(+), 4 deletions(-)
---
diff --git a/gi/pygi-invoke.c b/gi/pygi-invoke.c
index c93442a..78984e5 100644
--- a/gi/pygi-invoke.c
+++ b/gi/pygi-invoke.c
@@ -62,6 +62,12 @@ struct invocation_state
     PyObject  *return_value;
 
     GType      implementor_gtype;
+
+    /* hack to avoid treating C arrays as GArrays during free
+     * due to overly complicated array handling
+     * this will be removed when the new invoke branch is merged
+     */
+    gboolean c_arrays_are_wrapped;
 };
 
 static gboolean
@@ -121,6 +127,11 @@ _initialize_invocation_state (struct invocation_state *state,
     state->out_values = NULL;
     state->backup_args = NULL;
 
+    /* HACK: this gets marked FALSE whenever a C array in the args is
+     *       not wrapped by a GArray
+     */
+    state->c_arrays_are_wrapped = TRUE;
+
     return TRUE;
 }
 
@@ -556,6 +567,20 @@ _prepare_invocation_state (struct invocation_state *state,
                             (g_type_info_get_array_type (state->arg_type_infos[i]) == GI_ARRAY_TYPE_C)) {
                         state->args[i]->v_pointer = array->data;
 
+                        /* HACK: We have unwrapped a C array so
+                         *       set the state to reflect this.
+                         *       If there is an error between now
+                         *       and when we rewrap the array
+                         *       we will leak C arrays due to
+                         *       being in an inconsitant state.
+                         *       e.g. for interfaces with more
+                         *       than one C array argument, an
+                         *       error may occure when not all
+                         *       C arrays have been rewrapped.
+                         *       This will be removed once the invoke
+                         *       rewrite branch is merged.
+                         */
+                        state->c_arrays_are_wrapped = FALSE;
                         if (direction != GI_DIRECTION_INOUT || transfer != GI_TRANSFER_NOTHING) {
                             /* The array hasn't been referenced anywhere, so free it to avoid losing memory. */
                             g_array_free (array, FALSE);
@@ -851,6 +876,14 @@ _process_invocation_state (struct invocation_state *state,
 
         }
 
+        /* HACK: We rewrapped any C arrays above in a GArray so they are ok to
+         *       free as GArrays.  We will always leak C arrays if there is
+         *       an error before we reach this state as there is no easy way
+         *       to know which arrays were wrapped if there are more than one.
+         *       This will be removed with better array handling once merge
+         *       the invoke rewrite branch.
+         */
+        state->c_arrays_are_wrapped = TRUE;
         g_assert (state->n_return_values <= 1 || return_values_pos == state->n_return_values);
     }
 
@@ -900,13 +933,26 @@ _free_invocation_state (struct invocation_state *state)
                 backup_args_pos += 1;
             }
             if (state->args != NULL && state->args[i] != NULL) {
-                _pygi_argument_release (state->args[i], state->arg_type_infos[i],
-                                        transfer, direction);
-
                 type_tag = g_type_info_get_tag (state->arg_type_infos[i]);
+
+                if (type_tag == GI_TYPE_TAG_ARRAY &&
+                        (direction == GI_DIRECTION_IN || direction == GI_DIRECTION_INOUT) &&
+                        (g_type_info_get_array_type (state->arg_type_infos[i]) == GI_ARRAY_TYPE_C) &&
+                        !state->c_arrays_are_wrapped) {
+                    /* HACK: Noop - we are in an inconsitant state due to
+                     *       complex array handler so leak any C arrays
+                     *       as we don't know if we can free them safely.
+                     *       This will be removed when we merge the
+                     *       invoke rewrite branch.
+                     */
+                } else {
+                    _pygi_argument_release (state->args[i], state->arg_type_infos[i],
+                                            transfer, direction);
+                }
+
                 if (type_tag == GI_TYPE_TAG_ARRAY
                     && (direction != GI_DIRECTION_IN && transfer == GI_TRANSFER_NOTHING)) {
-                    /* We created a #GArray and it has not been released above, so free it. */
+                    /* We created an *out* #GArray and it has not been released above, so free it. */
                     state->args[i]->v_pointer = g_array_free (state->args[i]->v_pointer, FALSE);
                 }
             }
diff --git a/tests/test_gi.py b/tests/test_gi.py
index 4882a04..09046b1 100644
--- a/tests/test_gi.py
+++ b/tests/test_gi.py
@@ -1691,3 +1691,12 @@ class TestDir(unittest.TestCase):
         #        in the list:
         #
         # self.assertTrue('DoNotImportDummyTests' in list)
+
+class TestGErrorArrayInCrash(unittest.TestCase):
+    # Previously there was a bug in invoke, in which C arrays were unwrapped
+    # from inside GArrays to be passed to the C function. But when a GError was
+    # set, invoke would attempt to free the C array as if it were a GArray.
+    # This crash is only for C arrays. It does not happen for C functions which
+    # take in GArrays. See https://bugzilla.gnome.org/show_bug.cgi?id=642708
+    def test_gerror_array_in_crash(self):
+        self.assertRaises(GObject.GError, GIMarshallingTests.gerror_array_in, [1, 2, 3])



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]