Fix GArray, GList, GSList, and GHashTable marshaling leaks

Remove calling of cleanup code for transfer-everything modes by ensuring
cleanup_data is set to NULL in from_py marshalers. Use array and hash
table ref/unref functions for container transfer mode to ensure we have a
valid container ref after invoke and during from_py cleanup of contents.
Rework restrictions with to_py marshaling cleanup so we always unref the
container for transfer-everything and transfer-container modes.

https://bugzilla.gnome.org/show_bug.cgi?id=693402

2 files changed, 84 insertions, 47 deletions

diff --git a/gi/pygi-marshal-cleanup.c b/gi/pygi-marshal-cleanup.c
index 29ea6173..33d0339f 100644
--- a/gi/pygi-marshal-cleanup.c
+++ b/gi/pygi-marshal-cleanup.c
@@ -412,12 +412,11 @@ _pygi_marshal_cleanup_from_py_array (PyGIInvokeState *state,
              * passed back as cleanup_data
              */
             g_array_free (array_, arg_cache->transfer == GI_TRANSFER_NOTHING);
-        } else if (state->failed ||
-                   arg_cache->transfer == GI_TRANSFER_NOTHING) {
+        } else {
             if (array_ != NULL)
-                g_array_free (array_, TRUE);
+                g_array_unref (array_);
             else
-                g_ptr_array_free (ptr_array_, TRUE);
+                g_ptr_array_unref (ptr_array_);
         }
     }
 }
@@ -502,19 +501,12 @@ _pygi_marshal_cleanup_from_py_glist  (PyGIInvokeState *state,
             }
         }
 
-        if (state->failed ||
-               arg_cache->transfer == GI_TRANSFER_NOTHING ||
-                  arg_cache->transfer == GI_TRANSFER_CONTAINER) {
-            switch (arg_cache->type_tag) {
-                case GI_TYPE_TAG_GLIST:
-                    g_list_free ( (GList *)list_);
-                    break;
-                case GI_TYPE_TAG_GSLIST:
-                    g_slist_free (list_);
-                    break;
-                default:
-                    g_assert_not_reached();
-            }
+        if (arg_cache->type_tag == GI_TYPE_TAG_GLIST) {
+            g_list_free ( (GList *)list_);
+        } else if (arg_cache->type_tag == GI_TYPE_TAG_GSLIST) {
+            g_slist_free (list_);
+        } else {
+            g_assert_not_reached();
         }
     }
 }
@@ -527,7 +519,6 @@ _pygi_marshal_cleanup_to_py_glist (PyGIInvokeState *state,
                                    gboolean         was_processed)
 {
     PyGISequenceCache *sequence_cache = (PyGISequenceCache *)arg_cache;
-
     if (arg_cache->transfer == GI_TRANSFER_EVERYTHING ||
             arg_cache->transfer == GI_TRANSFER_CONTAINER) {
         GSList *list_ = (GSList *)data;
@@ -547,17 +538,12 @@ _pygi_marshal_cleanup_to_py_glist (PyGIInvokeState *state,
             }
         }
 
-        if (arg_cache->transfer == GI_TRANSFER_EVERYTHING) {
-            switch (arg_cache->type_tag) {
-                case GI_TYPE_TAG_GLIST:
-                    g_list_free ( (GList *)list_);
-                    break;
-                case GI_TYPE_TAG_GSLIST:
-                    g_slist_free (list_);
-                    break;
-                default:
-                    g_assert_not_reached();
-            }
+        if (arg_cache->type_tag == GI_TYPE_TAG_GLIST) {
+            g_list_free ( (GList *)list_);
+        } else if (arg_cache->type_tag == GI_TYPE_TAG_GSLIST) {
+            g_slist_free (list_);
+        } else {
+            g_assert_not_reached();
         }
     }
 }
@@ -607,11 +593,7 @@ _pygi_marshal_cleanup_from_py_ghash  (PyGIInvokeState *state,
             }
         }
 
-        if (state->failed ||
-               arg_cache->transfer == GI_TRANSFER_NOTHING ||
-                  arg_cache->transfer == GI_TRANSFER_CONTAINER)
-            g_hash_table_destroy (hash_);
-
+        g_hash_table_unref (hash_);
     }
 }
 
@@ -626,6 +608,6 @@ _pygi_marshal_cleanup_to_py_ghash (PyGIInvokeState *state,
         return;
 
     /* assume hashtable has boxed key and value */
-    if (arg_cache->transfer == GI_TRANSFER_EVERYTHING)
-        g_hash_table_destroy ( (GHashTable *)data);
+    if (arg_cache->transfer == GI_TRANSFER_EVERYTHING || arg_cache->transfer == GI_TRANSFER_CONTAINER)
+        g_hash_table_unref ( (GHashTable *)data);
 }
diff --git a/gi/pygi-marshal-from-py.c b/gi/pygi-marshal-from-py.c
index a2b58ccd..7bcba666 100644
--- a/gi/pygi-marshal-from-py.c
+++ b/gi/pygi-marshal-from-py.c
@@ -764,7 +764,7 @@ _pygi_marshal_from_py_array (PyGIInvokeState   *state,
     item_size = sequence_cache->item_size;
     is_ptr_array = (sequence_cache->array_type == GI_ARRAY_TYPE_PTR_ARRAY);
     if (is_ptr_array) {
-        array_ = (GArray *)g_ptr_array_new ();
+        array_ = (GArray *)g_ptr_array_sized_new (length);
     } else {
         array_ = g_array_sized_new (sequence_cache->is_zero_terminated,
                                     TRUE,
@@ -938,17 +938,35 @@ array_success:
     }
 
     if (sequence_cache->array_type == GI_ARRAY_TYPE_C) {
+        /* In the case of GI_ARRAY_C, we give the data directly as the argument
+         * but keep the array_ wrapper as cleanup data so we don't have to find
+         * it's length again.
+         */
         arg->v_pointer = array_->data;
+
+        if (arg_cache->transfer == GI_TRANSFER_EVERYTHING) {
+            g_array_free (array_, FALSE);
+            *cleanup_data = NULL;
+        } else {
+            *cleanup_data = array_;
+        }
     } else {
         arg->v_pointer = array_;
-    }
 
-    /* In all cases give the array object back as cleanup data.
-     * In the case of GI_ARRAY_C, we give the data directly as the argument
-     * but keep the array_ wrapper as cleanup data so we don't have to find
-     * it's length again.
-     */
-    *cleanup_data = array_;
+        if (arg_cache->transfer == GI_TRANSFER_NOTHING) {
+            /* Free everything in cleanup. */
+            *cleanup_data = array_;
+        } else if (arg_cache->transfer == GI_TRANSFER_CONTAINER) {
+            /* Make a shallow copy so we can free the elements later in cleanup
+             * because it is possible invoke will free the list before our cleanup. */
+            *cleanup_data = is_ptr_array ?
+                    (gpointer)g_ptr_array_ref ((GPtrArray *)array_) :
+                    (gpointer)g_array_ref (array_);
+        } else { /* GI_TRANSFER_EVERYTHING */
+            /* No cleanup, everything is given to the callee. */
+            *cleanup_data = NULL;
+        }
+    }
 
     return TRUE;
 }
@@ -1023,7 +1041,18 @@ err:
     }
 
     arg->v_pointer = g_list_reverse (list_);
-    *cleanup_data = arg->v_pointer;
+
+    if (arg_cache->transfer == GI_TRANSFER_NOTHING) {
+        /* Free everything in cleanup. */
+        *cleanup_data = arg->v_pointer;
+    } else if (arg_cache->transfer == GI_TRANSFER_CONTAINER) {
+        /* Make a shallow copy so we can free the elements later in cleanup
+         * because it is possible invoke will free the list before our cleanup. */
+        *cleanup_data = g_list_copy (arg->v_pointer);
+    } else { /* GI_TRANSFER_EVERYTHING */
+        /* No cleanup, everything is given to the callee. */
+        *cleanup_data = NULL;
+    }
     return TRUE;
 }
 
@@ -1097,7 +1126,19 @@ err:
     }
 
     arg->v_pointer = g_slist_reverse (list_);
-    *cleanup_data = arg->v_pointer;
+
+    if (arg_cache->transfer == GI_TRANSFER_NOTHING) {
+        /* Free everything in cleanup. */
+        *cleanup_data = arg->v_pointer;
+    } else if (arg_cache->transfer == GI_TRANSFER_CONTAINER) {
+        /* Make a shallow copy so we can free the elements later in cleanup
+         * because it is possible invoke will free the list before our cleanup. */
+        *cleanup_data = g_slist_copy (arg->v_pointer);
+    } else { /* GI_TRANSFER_EVERYTHING */
+        /* No cleanup, everything is given to the callee. */
+        *cleanup_data = NULL;
+    }
+
     return TRUE;
 }
 
@@ -1209,7 +1250,21 @@ err:
     }
 
     arg->v_pointer = hash_;
-    *cleanup_data = arg->v_pointer;
+
+    if (arg_cache->transfer == GI_TRANSFER_NOTHING) {
+        /* Free everything in cleanup. */
+        *cleanup_data = arg->v_pointer;
+    } else if (arg_cache->transfer == GI_TRANSFER_CONTAINER) {
+        /* Make a shallow copy so we can free the elements later in cleanup
+         * because it is possible invoke will free the list before our cleanup. */
+        *cleanup_data = g_hash_table_ref (arg->v_pointer);
+    } else { /* GI_TRANSFER_EVERYTHING */
+        /* No cleanup, everything is given to the callee.
+         * Note that the keys and values will leak for transfer everything because
+         * we do not use g_hash_table_new_full and set key/value_destroy_func. */
+        *cleanup_data = NULL;
+    }
+
     return TRUE;
 }