[pygobject] Fix GValue array marshaling leaks and crash fallout
- From: Simon Feltman <sfeltman src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [pygobject] Fix GValue array marshaling leaks and crash fallout
- Date: Tue, 8 Oct 2013 01:19:45 +0000 (UTC)
commit 4623caa71c54958ab821db27a9eff2790acb3975
Author: Simon Feltman <sfeltman src gnome org>
Date: Sat Oct 5 17:00:54 2013 -0700
Fix GValue array marshaling leaks and crash fallout
* Decrement references for results of PySequence_GetItem. There were a few
places we were not decrementing the Python reference, leaking the value.
* Add tracking of Python arguments with recursive marshaling cleanup. This
allows arrays of GValues which have been coerced from Python types to be
properly free'd (also fixes bug 703662).
* Use g_variant_ref for variant arguments marked as transfer everything.
This fixes double free's caused by the decrementing of PySequence_GetItem
results.
https://bugzilla.gnome.org/show_bug.cgi?id=693402
gi/pygi-cache.h | 1 +
gi/pygi-invoke.c | 1 +
gi/pygi-marshal-cleanup.c | 50 +++++++++++++++++++++++++++++++++++++++-----
gi/pygi-marshal-cleanup.h | 14 ++++++++++++
gi/pygi-marshal-from-py.c | 20 +++++++++++++++--
gi/pygi-marshal-to-py.c | 1 +
6 files changed, 78 insertions(+), 9 deletions(-)
---
diff --git a/gi/pygi-cache.h b/gi/pygi-cache.h
index 65e2de5..24a8406 100644
--- a/gi/pygi-cache.h
+++ b/gi/pygi-cache.h
@@ -45,6 +45,7 @@ typedef PyObject *(*PyGIMarshalToPyFunc) (PyGIInvokeState *state,
typedef void (*PyGIMarshalCleanupFunc) (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *py_arg, /* always NULL for to_py cleanup */
gpointer data,
gboolean was_processed);
diff --git a/gi/pygi-invoke.c b/gi/pygi-invoke.c
index caa72be..2303638 100644
--- a/gi/pygi-invoke.c
+++ b/gi/pygi-invoke.c
@@ -579,6 +579,7 @@ _invoke_marshal_out_args (PyGIInvokeState *state, PyGICallableCache *cache)
if (to_py_cleanup != NULL)
to_py_cleanup ( state,
cache->return_cache,
+ NULL,
&state->return_arg,
FALSE);
}
diff --git a/gi/pygi-marshal-cleanup.c b/gi/pygi-marshal-cleanup.c
index a877306..c5dfc6a 100644
--- a/gi/pygi-marshal-cleanup.c
+++ b/gi/pygi-marshal-cleanup.c
@@ -24,6 +24,7 @@
static inline void
_cleanup_caller_allocates (PyGIInvokeState *state,
PyGIArgCache *cache,
+ PyObject *py_obj,
gpointer data,
gboolean was_processed)
{
@@ -97,16 +98,18 @@ pygi_marshal_cleanup_args_from_py_marshal_success (PyGIInvokeState *state,
for (i = 0; i < _pygi_callable_cache_args_len (cache); i++) {
PyGIArgCache *arg_cache = _pygi_callable_cache_get_arg (cache, i);
PyGIMarshalCleanupFunc cleanup_func = arg_cache->from_py_cleanup;
+ PyObject *py_arg = PyTuple_GET_ITEM (state->py_in_args,
+ arg_cache->py_arg_index);
if (cleanup_func &&
arg_cache->direction == PYGI_DIRECTION_FROM_PYTHON &&
state->args[i]->v_pointer != NULL)
- cleanup_func (state, arg_cache, state->args[i]->v_pointer, TRUE);
+ cleanup_func (state, arg_cache, py_arg, state->args[i]->v_pointer, TRUE);
if (cleanup_func &&
arg_cache->direction == PYGI_DIRECTION_BIDIRECTIONAL &&
state->args_data[i] != NULL) {
- cleanup_func (state, arg_cache, state->args_data[i], TRUE);
+ cleanup_func (state, arg_cache, py_arg, state->args_data[i], TRUE);
}
}
}
@@ -122,6 +125,7 @@ pygi_marshal_cleanup_args_to_py_marshal_success (PyGIInvokeState *state,
if (cleanup_func && state->return_arg.v_pointer != NULL)
cleanup_func (state,
cache->return_cache,
+ NULL,
state->return_arg.v_pointer,
TRUE);
}
@@ -136,11 +140,13 @@ pygi_marshal_cleanup_args_to_py_marshal_success (PyGIInvokeState *state,
if (cleanup_func != NULL && data != NULL)
cleanup_func (state,
arg_cache,
+ NULL,
data,
TRUE);
else if (arg_cache->is_caller_allocates && data != NULL) {
_cleanup_caller_allocates (state,
arg_cache,
+ NULL,
data,
TRUE);
}
@@ -162,18 +168,22 @@ pygi_marshal_cleanup_args_from_py_parameter_fail (PyGIInvokeState *state,
PyGIArgCache *arg_cache = _pygi_callable_cache_get_arg (cache, i);
PyGIMarshalCleanupFunc cleanup_func = arg_cache->from_py_cleanup;
gpointer data = state->args[i]->v_pointer;
+ PyObject *py_arg = PyTuple_GET_ITEM (state->py_in_args,
+ arg_cache->py_arg_index);
if (cleanup_func &&
arg_cache->direction == PYGI_DIRECTION_FROM_PYTHON &&
data != NULL) {
cleanup_func (state,
arg_cache,
+ py_arg,
data,
i < failed_arg_index);
} else if (arg_cache->is_caller_allocates && data != NULL) {
_cleanup_caller_allocates (state,
arg_cache,
+ py_arg,
data,
FALSE);
}
@@ -198,6 +208,7 @@ pygi_marshal_cleanup_args_to_py_parameter_fail (PyGIInvokeState *state,
void
_pygi_marshal_cleanup_from_py_utf8 (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *py_arg,
gpointer data,
gboolean was_processed)
{
@@ -210,6 +221,7 @@ _pygi_marshal_cleanup_from_py_utf8 (PyGIInvokeState *state,
void
_pygi_marshal_cleanup_to_py_utf8 (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *dummy,
gpointer data,
gboolean was_processed)
{
@@ -223,6 +235,7 @@ _pygi_marshal_cleanup_to_py_utf8 (PyGIInvokeState *state,
void
_pygi_marshal_cleanup_from_py_interface_object (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *py_arg,
gpointer data,
gboolean was_processed)
{
@@ -236,6 +249,7 @@ _pygi_marshal_cleanup_from_py_interface_object (PyGIInvokeState *state,
void
_pygi_marshal_cleanup_to_py_interface_object (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *dummy,
gpointer data,
gboolean was_processed)
{
@@ -249,6 +263,7 @@ _pygi_marshal_cleanup_to_py_interface_object (PyGIInvokeState *state,
void
_pygi_marshal_cleanup_from_py_interface_callback (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *py_arg,
gpointer data,
gboolean was_processed)
{
@@ -262,15 +277,18 @@ _pygi_marshal_cleanup_from_py_interface_callback (PyGIInvokeState *state,
void
_pygi_marshal_cleanup_from_py_interface_struct_gvalue (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *py_arg,
gpointer data,
gboolean was_processed)
{
- if (was_processed) {
- PyObject *py_arg = PyTuple_GET_ITEM (state->py_in_args,
- arg_cache->py_arg_index);
+ /* Note py_arg can be NULL for hash table which is a bug. */
+ if (was_processed && py_arg != NULL) {
GType py_object_type =
pyg_type_from_object_strict ( (PyObject *) py_arg->ob_type, FALSE);
+ /* When a GValue was not passed, it means the marshalers created a new
+ * one to pass in, clean this up.
+ */
if (py_object_type != G_TYPE_VALUE) {
g_value_unset ((GValue *) data);
g_slice_free (GValue, data);
@@ -281,6 +299,7 @@ _pygi_marshal_cleanup_from_py_interface_struct_gvalue (PyGIInvokeState *state,
void
_pygi_marshal_cleanup_from_py_interface_struct_foreign (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *py_arg,
gpointer data,
gboolean was_processed)
{
@@ -293,6 +312,7 @@ _pygi_marshal_cleanup_from_py_interface_struct_foreign (PyGIInvokeState *state,
void
_pygi_marshal_cleanup_to_py_interface_struct_foreign (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *dummy,
gpointer data,
gboolean was_processed)
{
@@ -336,6 +356,7 @@ _wrap_c_array (PyGIInvokeState *state,
void
_pygi_marshal_cleanup_from_py_array (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *py_arg,
gpointer data,
gboolean was_processed)
{
@@ -367,6 +388,7 @@ _pygi_marshal_cleanup_from_py_array (PyGIInvokeState *state,
for (i = 0; i < len; i++) {
gpointer item;
+ PyObject *py_item = NULL;
/* case 1: GPtrArray */
if (ptr_array_ != NULL)
@@ -387,7 +409,9 @@ _pygi_marshal_cleanup_from_py_array (PyGIInvokeState *state,
}
}
- cleanup_func (state, sequence_cache->item_cache, item, TRUE);
+ py_item = PySequence_GetItem (py_arg, i);
+ cleanup_func (state, sequence_cache->item_cache, py_item, item, TRUE);
+ Py_XDECREF (py_item);
}
}
@@ -407,6 +431,7 @@ _pygi_marshal_cleanup_from_py_array (PyGIInvokeState *state,
void
_pygi_marshal_cleanup_to_py_array (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *dummy,
gpointer data,
gboolean was_processed)
{
@@ -438,6 +463,7 @@ _pygi_marshal_cleanup_to_py_array (PyGIInvokeState *state,
for (i = 0; i < len; i++) {
cleanup_func (state,
sequence_cache->item_cache,
+ NULL,
(array_ != NULL) ? g_array_index (array_, gpointer, i) : g_ptr_array_index
(ptr_array_, i),
was_processed);
}
@@ -453,6 +479,7 @@ _pygi_marshal_cleanup_to_py_array (PyGIInvokeState *state,
void
_pygi_marshal_cleanup_from_py_glist (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *py_arg,
gpointer data,
gboolean was_processed)
{
@@ -467,12 +494,17 @@ _pygi_marshal_cleanup_from_py_glist (PyGIInvokeState *state,
PyGIMarshalCleanupFunc cleanup_func =
sequence_cache->item_cache->from_py_cleanup;
GSList *node = list_;
+ gsize i = 0;
while (node != NULL) {
+ PyObject *py_item = PySequence_GetItem (py_arg, i);
cleanup_func (state,
sequence_cache->item_cache,
+ py_item,
node->data,
TRUE);
+ Py_XDECREF (py_item);
node = node->next;
+ i++;
}
}
@@ -496,6 +528,7 @@ _pygi_marshal_cleanup_from_py_glist (PyGIInvokeState *state,
void
_pygi_marshal_cleanup_to_py_glist (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *dummy,
gpointer data,
gboolean was_processed)
{
@@ -513,6 +546,7 @@ _pygi_marshal_cleanup_to_py_glist (PyGIInvokeState *state,
while (node != NULL) {
cleanup_func (state,
sequence_cache->item_cache,
+ NULL,
node->data,
was_processed);
node = node->next;
@@ -537,6 +571,7 @@ _pygi_marshal_cleanup_to_py_glist (PyGIInvokeState *state,
void
_pygi_marshal_cleanup_from_py_ghash (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *py_arg,
gpointer data,
gboolean was_processed)
{
@@ -566,11 +601,13 @@ _pygi_marshal_cleanup_from_py_ghash (PyGIInvokeState *state,
if (key != NULL && key_cleanup_func != NULL)
key_cleanup_func (state,
hash_cache->key_cache,
+ NULL,
key,
TRUE);
if (value != NULL && value_cleanup_func != NULL)
value_cleanup_func (state,
hash_cache->value_cache,
+ NULL,
value,
TRUE);
}
@@ -587,6 +624,7 @@ _pygi_marshal_cleanup_from_py_ghash (PyGIInvokeState *state,
void
_pygi_marshal_cleanup_to_py_ghash (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *dummy,
gpointer data,
gboolean was_processed)
{
diff --git a/gi/pygi-marshal-cleanup.h b/gi/pygi-marshal-cleanup.h
index 234e49c..3acfbeb 100644
--- a/gi/pygi-marshal-cleanup.h
+++ b/gi/pygi-marshal-cleanup.h
@@ -42,58 +42,72 @@ void pygi_marshal_cleanup_args_to_py_parameter_fail (PyGIInvokeState *state,
void _pygi_marshal_cleanup_from_py_utf8 (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *py_arg,
gpointer data,
gboolean was_processed);
void _pygi_marshal_cleanup_to_py_utf8 (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *dummy,
gpointer data,
gboolean was_processed);
void _pygi_marshal_cleanup_from_py_interface_struct_gvalue (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *py_arg,
gpointer data,
gboolean was_processed);
void _pygi_marshal_cleanup_from_py_interface_struct_foreign (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *py_arg,
gpointer data,
gboolean was_processed);
void _pygi_marshal_cleanup_to_py_interface_struct_foreign (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *dummy,
gpointer data,
gboolean was_processed);
void _pygi_marshal_cleanup_from_py_interface_object (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *py_arg,
gpointer data,
gboolean was_processed);
void _pygi_marshal_cleanup_to_py_interface_object (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *dummy,
gpointer data,
gboolean was_processed);
void _pygi_marshal_cleanup_from_py_interface_callback (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *py_arg,
gpointer data,
gboolean was_processed);
void _pygi_marshal_cleanup_from_py_array (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *py_arg,
gpointer data,
gboolean was_processed);
void _pygi_marshal_cleanup_to_py_array (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *dummy,
gpointer data,
gboolean was_processed);
void _pygi_marshal_cleanup_from_py_glist (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *py_arg,
gpointer data,
gboolean was_processed);
void _pygi_marshal_cleanup_to_py_glist (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *dummy,
gpointer data,
gboolean was_processed);
void _pygi_marshal_cleanup_from_py_ghash (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *py_arg,
gpointer data,
gboolean was_processed);
void _pygi_marshal_cleanup_to_py_ghash (PyGIInvokeState *state,
PyGIArgCache *arg_cache,
+ PyObject *dummy,
gpointer data,
gboolean was_processed);
G_END_DECLS
diff --git a/gi/pygi-marshal-from-py.c b/gi/pygi-marshal-from-py.c
index 73356b1..586a87c 100644
--- a/gi/pygi-marshal-from-py.c
+++ b/gi/pygi-marshal-from-py.c
@@ -788,9 +788,12 @@ _pygi_marshal_from_py_array (PyGIInvokeState *state,
callable_cache,
sequence_cache->item_cache,
py_item,
- &item))
+ &item)) {
+ Py_DECREF (py_item);
goto err;
+ }
+ Py_DECREF (py_item);
/* FIXME: it is much more efficent to have seperate marshaller
* for ptr arrays than doing the evaluation
* and casting each loop iteration
@@ -827,7 +830,7 @@ _pygi_marshal_from_py_array (PyGIInvokeState *state,
/* we free the original copy already, the new one is a plain struct
* in an array. _pygi_marshal_cleanup_from_py_array() does not free it again */
if (from_py_cleanup)
- from_py_cleanup (state, item_arg_cache, item.v_pointer, TRUE);
+ from_py_cleanup (state, item_arg_cache, py_item, item.v_pointer, TRUE);
} else if (!is_boxed) {
/* HACK: Gdk.Atom is merely an integer wrapped in a pointer,
* so we must not dereference it; just copy the pointer
@@ -841,7 +844,7 @@ _pygi_marshal_from_py_array (PyGIInvokeState *state,
memcpy (array_->data + (i * item_size), item.v_pointer, item_size);
if (from_py_cleanup)
- from_py_cleanup (state, item_arg_cache, item.v_pointer, TRUE);
+ from_py_cleanup (state, item_arg_cache, py_item, item.v_pointer, TRUE);
}
} else if (is_boxed && !item_iface_cache->arg_cache.is_pointer) {
/* The array elements are not expected to be pointers, but the
@@ -860,6 +863,7 @@ _pygi_marshal_from_py_array (PyGIInvokeState *state,
} else {
g_array_insert_val (array_, i, item);
}
+
continue;
err:
if (sequence_cache->item_cache->from_py_cleanup != NULL) {
@@ -868,10 +872,13 @@ err:
sequence_cache->item_cache->from_py_cleanup;
for(j = 0; j < i; j++) {
+ PyObject *py_item = PySequence_GetItem (py_arg, j);
cleanup_func (state,
sequence_cache->item_cache,
+ py_item,
g_array_index (array_, gpointer, j),
TRUE);
+ Py_DECREF (py_item);
}
}
@@ -975,6 +982,7 @@ _pygi_marshal_from_py_glist (PyGIInvokeState *state,
&item))
goto err;
+ Py_DECREF (py_item);
list_ = g_list_prepend (list_, _pygi_arg_to_hash_pointer (&item,
sequence_cache->item_cache->type_tag));
continue;
err:
@@ -983,6 +991,7 @@ err:
PyGIMarshalCleanupFunc cleanup = sequence_cache->item_cache->from_py_cleanup;
}
*/
+ Py_DECREF (py_item);
g_list_free (list_);
_PyGI_ERROR_PREFIX ("Item %i: ", i);
return FALSE;
@@ -1042,6 +1051,7 @@ _pygi_marshal_from_py_gslist (PyGIInvokeState *state,
&item))
goto err;
+ Py_DECREF (py_item);
list_ = g_slist_prepend (list_, _pygi_arg_to_hash_pointer (&item,
sequence_cache->item_cache->type_tag));
continue;
err:
@@ -1051,6 +1061,7 @@ err:
}
*/
+ Py_DECREF (py_item);
g_slist_free (list_);
_PyGI_ERROR_PREFIX ("Item %i: ", i);
return FALSE;
@@ -1757,6 +1768,9 @@ _pygi_marshal_from_py_interface_struct (PyObject *py_arg,
return FALSE;
}
arg->v_pointer = pyg_pointer_get (py_arg, void);
+ if (transfer == GI_TRANSFER_EVERYTHING) {
+ g_variant_ref ((GVariant *)arg->v_pointer);
+ }
} else {
PyErr_Format (PyExc_NotImplementedError,
diff --git a/gi/pygi-marshal-to-py.c b/gi/pygi-marshal-to-py.c
index c9fbce9..2dc7f71 100644
--- a/gi/pygi-marshal-to-py.c
+++ b/gi/pygi-marshal-to-py.c
@@ -425,6 +425,7 @@ err:
for (j = processed_items; j < array_->len; j++) {
cleanup_func (state,
seq_cache->item_cache,
+ NULL,
g_array_index (array_, gpointer, j),
FALSE);
}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
