On 10/05/11 16:08, Bert Huijben wrote:
> The python bindings need it, so I would say: Add it there.
The attached patch fixes the FD leak entirely within the Python bindings.
> (And maybe make it easier to use from other bindings when it proves useful.
> That is why I suggested adding the function in svn_stream.h)
I'm not familiar enough with the use of streams in subversion to be able
to argue for any particular approach; so I hope that by keeping this
entirely to the Python bindings it may be committed.
I expect we'll use this patch (today's version) in our production
service very soon.
Nick
--
Sorry for this disclaimer:
Think green - keep it on the screen.
This e-mail and any attachment is for authorised use by the intended
recipient(s) only. It may contain proprietary material, confidential
information and/or be subject to legal privilege. It should not be copied,
disclosed to, retained or used by, any other party. If you are not an intended
recipient then please promptly delete this e-mail and any attachment and all
copies and inform the sender. Thank you.
>From 0caa8888cfe898930ecee2a9e97a66d3cbbfb5ac Mon Sep 17 00:00:00 2001
From: Nick Piper <nick.pi...@logica.com>
Date: Wed, 11 May 2011 10:51:33 +0100
Subject: [PATCH] Ensure that Py_DECREF(py_io) is called before the pool containing the stream is cleared and close the Python 'file' when the stream is closed.
Note that closing the stream does not remove our reference to the underlying python object. I believe this more closely matches normal Python symantics.
---
.../swig/python/libsvn_swig_py/swigutil_py.c | 26 ++++++++++++++++++-
1 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c b/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
index d8b9c08..19c8536 100644
--- a/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
+++ b/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
@@ -2107,11 +2107,32 @@ write_handler_pyio(void *baton, const char *data, apr_size_t *len)
static svn_error_t *
close_handler_pyio(void *baton)
{
+ PyObject *result;
PyObject *py_io = baton;
+ svn_error_t *err = SVN_NO_ERROR;
+
+ if (py_io != Py_None)
+ {
+ svn_swig_py_acquire_py_lock();
+ if ((result = PyObject_CallMethod(py_io, (char *)"close", (char *) NULL)) == NULL)
+ {
+ err = callback_exception_error();
+ }
+ Py_XDECREF(result);
+ svn_swig_py_release_py_lock();
+ }
+
+ return err;
+}
+
+
+static apr_status_t
+svn_swig_py_stream_destroy(void *py_io)
+{
svn_swig_py_acquire_py_lock();
- Py_DECREF(py_io);
+ Py_DECREF((PyObject *) py_io);
svn_swig_py_release_py_lock();
- return SVN_NO_ERROR;
+ return APR_SUCCESS;
}
svn_stream_t *
@@ -2124,6 +2145,7 @@ svn_swig_py_make_stream(PyObject *py_io, apr_pool_t *pool)
svn_stream_set_write(stream, write_handler_pyio);
svn_stream_set_close(stream, close_handler_pyio);
Py_INCREF(py_io);
+ apr_pool_cleanup_register(pool, py_io, svn_swig_py_stream_destroy, apr_pool_cleanup_null);
return stream;
}
--
1.7.1
