pygobject r999 - in trunk: . gio

[paulp svn gnome org]

Author: paulp
Date: Thu Jan 29 19:39:53 2009
New Revision: 999
URL: http://svn.gnome.org/viewvc/pygobject?rev=999&view=rev

Log:
2009-01-29  Paul Pogonyshev  <pogonyshev gmx net>

	Bug 567792 â gio.InputStream.read_async can cause memory
	corruption

	* gio/gio.override (py_decref_callback): Remove (unused now).
	(pygio_notify_allocate_buffer, pygio_notify_attach_to_result)
	(pygio_notify_get_attached): New functions.
	(async_result_callback_marshal): Attach to the result object if
	asked and then don't free self.  Fix reference leak (spotted by
	Gustavo).

	* gio/ginputstream.override (async_result_callback_marshal_read):
	Remove.  Use new attachment functionality in
	async_result_callback_marshal() instead.
	(_wrap_g_input_stream_read_async): Use the new functions (also
	those mentioned in the cleanup log below).
	(_wrap_g_input_stream_read_finish): Use the new functions.


Modified:
   trunk/ChangeLog
   trunk/gio/ginputstream.override
   trunk/gio/gio.override

Modified: trunk/gio/ginputstream.override
==============================================================================
--- trunk/gio/ginputstream.override	(original)
+++ trunk/gio/ginputstream.override	Thu Jan 29 19:39:53 2009
@@ -23,55 +23,6 @@
 headers
 #define BUFSIZE 8192
 
-typedef struct {
-    PyObject *callback;
-    PyObject *data;
-    guchar *buffer;
-} PyGIONotifyRead;
-
-static void
-async_result_callback_marshal_read(GObject *source_object,
-                                   GAsyncResult *result,
-                                   PyGIONotifyRead *notify)
-{
-    PyObject *ret;
-    PyGILState_STATE state;
-    static GQuark quark = 0;
-
-    state = pyg_gil_state_ensure();
-
-    /* buffer is only used by read_async */
-    if (notify->buffer) {
-        if (!quark)
-            quark = g_quark_from_string("pygio::buffer");
-        g_object_set_qdata_full(G_OBJECT(result), quark,
-                                notify->buffer, py_decref_callback);
-    }
-
-    if (notify->data)
-        ret = PyEval_CallFunction(notify->callback, "(OOO)",
-                                  pygobject_new(source_object),
-                                  pygobject_new((GObject *)result),
-                                  notify->data);
-    else
-        ret = PyObject_CallFunction(notify->callback, "(OO)",
-                                    pygobject_new(source_object),
-                                    pygobject_new((GObject *)result));
-
-    if (ret == NULL)
-        {
-            PyErr_Print();
-            PyErr_Clear();
-        }
-
-    Py_XDECREF(ret);
-
-    Py_DECREF(notify->callback);
-    Py_XDECREF(notify->data);
-    g_slice_free(PyGIONotifyRead, notify);
-
-    pyg_gil_state_release(state);
-}
 %%
 override g_input_stream_read kwargs
 static PyObject *
@@ -224,9 +175,9 @@
     int io_priority = G_PRIORITY_DEFAULT;
     PyGObject *pycancellable = NULL;
     GCancellable *cancellable;
-    PyGIONotifyRead *notify;
+    PyGIONotify *notify;
 
-    notify = g_slice_new0(PyGIONotifyRead);
+    notify = pygio_notify_new();
 
     if (!PyArg_ParseTupleAndKeywords(args, kwargs,
                                      "lO|iOO:InputStream.read_async",
@@ -236,37 +187,34 @@
                                      &io_priority,
                                      &pycancellable,
                                      &notify->data))
-        {
-            g_slice_free(PyGIONotifyRead, notify);
-            return NULL;
-        }
+        goto error;
 
-    if (!PyCallable_Check(notify->callback))
-        {
-            PyErr_SetString(PyExc_TypeError, "callback argument not callable");
-            g_slice_free(PyGIONotifyRead, notify);
-            return NULL;
-        }
-    Py_INCREF(notify->callback);
-    Py_XINCREF(notify->data);
+    if (!pygio_notify_callback_is_valid(notify))
+        goto error;
 
     if (!pygio_check_cancellable(pycancellable, &cancellable))
-        return NULL;
+        goto error;
 
-    notify->buffer = g_malloc(count);
-    if (notify->buffer == NULL)
-        return NULL;
+    if (!pygio_notify_allocate_buffer(notify, count))
+        goto error;
+
+    pygio_notify_reference_callback(notify);
+    pygio_notify_attach_to_result(notify);
 
     g_input_stream_read_async(G_INPUT_STREAM(self->obj),
                               notify->buffer,
-                              count,
+                              notify->buffer_size,
                               io_priority,
                               cancellable,
-                              (GAsyncReadyCallback)async_result_callback_marshal_read,
+                              (GAsyncReadyCallback) async_result_callback_marshal,
                               notify);
 
     Py_INCREF(Py_None);
     return Py_None;
+
+ error:
+    pygio_notify_free(notify);
+    return NULL;
 }
 %%
 override g_input_stream_read_finish kwargs
@@ -278,31 +226,25 @@
     static char *kwlist[] = { "result", NULL };
     PyGObject *result;
     GError *error = NULL;
-    static GQuark quark = 0;
-    gchar *buffer;
     Py_ssize_t bytesread;
-
-    if (!quark)
-        quark = g_quark_from_string("pygio::buffer");
+    PyGIONotify *notify;
 
     if (!PyArg_ParseTupleAndKeywords(args, kwargs,
                                      "O!:GInputStream.read_finish",
                                      kwlist, &PyGAsyncResult_Type, &result))
         return NULL;
 
-
     bytesread = g_input_stream_read_finish(G_INPUT_STREAM(self->obj),
                                            G_ASYNC_RESULT(result->obj), &error);
 
     if (pyg_error_check(&error))
         return NULL;
 
-    if (bytesread == 0) {
+    if (bytesread == 0)
         return PyString_FromString("");
-    }
 
-    buffer = g_object_get_qdata(G_OBJECT(result->obj), quark);
-    return PyString_FromStringAndSize(buffer, bytesread);
+    notify = pygio_notify_get_attached(result);
+    return PyString_FromStringAndSize(notify->buffer, bytesread);
 }
 %%
 override g_input_stream_close_async kwargs

Modified: trunk/gio/gio.override
==============================================================================
--- trunk/gio/gio.override	(original)
+++ trunk/gio/gio.override	Thu Jan 29 19:39:53 2009
@@ -37,23 +37,18 @@
     gboolean  referenced;
     PyObject *callback;
     PyObject *data;
+    gboolean  attach_self;
     gpointer  buffer;
     gsize     buffer_size;
 } PyGIONotify;
 
-static void
-py_decref_callback (gpointer data)
+static GQuark
+pygio_notify_get_internal_quark(void)
 {
-    Py_DECREF((PyObject*)data);
-}
-
-static void
-pygio_notify_copy_buffer(PyGIONotify *notify, gpointer buffer, gsize buffer_size)
-{
-    if (buffer_size > 0) {
-	notify->buffer = g_slice_copy(buffer_size, buffer);
-	notify->buffer_size = buffer_size;
-    }
+    static GQuark quark = 0;
+    if (!quark)
+        quark = g_quark_from_string("pygio::notify");
+    return quark;
 }
 
 static PyGIONotify *
@@ -109,6 +104,43 @@
 }
 
 static void
+pygio_notify_copy_buffer(PyGIONotify *notify, gpointer buffer, gsize buffer_size)
+{
+    if (buffer_size > 0) {
+	notify->buffer = g_slice_copy(buffer_size, buffer);
+	notify->buffer_size = buffer_size;
+    }
+}
+
+static gboolean
+pygio_notify_allocate_buffer(PyGIONotify *notify, gsize buffer_size)
+{
+    if (buffer_size > 0) {
+        notify->buffer = g_slice_alloc(buffer_size);
+        if (!notify->buffer) {
+            PyErr_Format(PyExc_MemoryError, "failed to allocate %d bytes", buffer_size);
+            return FALSE;
+        }
+
+        notify->buffer_size = buffer_size;
+    }
+
+    return TRUE;
+}
+
+static void
+pygio_notify_attach_to_result(PyGIONotify *notify)
+{
+    notify->attach_self = TRUE;
+}
+
+static PyGIONotify *
+pygio_notify_get_attached(PyGObject *result)
+{
+    return g_object_get_qdata(G_OBJECT(result->obj), pygio_notify_get_internal_quark());
+}
+
+static void
 pygio_notify_free(PyGIONotify *notify)
 {
     if (notify) {
@@ -137,13 +169,18 @@
     if (!notify->referenced)
         g_warning("pygio_notify_reference_callback() hasn't been called before using the structure");
 
+    if (notify->attach_self) {
+        g_object_set_qdata_full(G_OBJECT(result), pygio_notify_get_internal_quark(),
+                                notify, (GDestroyNotify) pygio_notify_free);
+    }
+
     if (notify->data)
-	ret = PyEval_CallFunction(notify->callback, "(OOO)",
+	ret = PyEval_CallFunction(notify->callback, "NNO",
 				  pygobject_new(source_object),
 				  pygobject_new((GObject *)result),
 				  notify->data);
     else
-	ret = PyObject_CallFunction(notify->callback, "(OO)",
+	ret = PyObject_CallFunction(notify->callback, "NN",
 				    pygobject_new(source_object),
 				    pygobject_new((GObject *)result));
 
@@ -153,7 +190,11 @@
     }
 
     Py_XDECREF(ret);
-    pygio_notify_free(notify);
+
+    /* Otherwise the structure is attached to 'result' and will be
+     * freed when that object dies. */
+    if (!notify->attach_self)
+        pygio_notify_free(notify);
 
     pyg_gil_state_release(state);
 }