[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: swig, python, lifetimes

From: David James <james_at_cs.toronto.edu>
Date: Sat, 17 May 2008 14:35:49 -0700

On Thu, May 15, 2008 at 12:52 PM, David Glasser
<glasser_at_davidglasser.net> wrote:
> So, let's say I write the following swig-py code:
> def foo():
> editor = MyEditor()
> e, eb = svn.delta.make_editor(editor)
> r, rb = svn.ra.do_update2(ra, ..., e, eb, ...)
> # do stuff with r, rb
> That's fine. Now let's say I do this:
> def DoUpdate(editor):
> e, eb = svn.delta.make_editor(editor)
> r, rb = svn.ra.do_update2(ra, ..., e, eb, ...)
> return r, rb
> def foo():
> editor = MyEditor()
> r, rb = DoUpdate(editor)
> # Do stuff with r, rb.
> This can lead to segfaults. Why? Because e and eb are destroyed at
> the end of DoUpdate, even though the C reporter in r and rb is holding
> onto references to it.
> In my particular code, I was actually wrapping r and rb in a Reporter
> class anyway, so I just added "reporter._hold_refs_ = [e, eb]".
> That's kind of hacky, though.
> I talked to epg about this for a while. Ideally what one would want
> would be for the do_update2 wrapper to do an incref on its e/eb
> arguments, and for something (__del__ on r, say) to decref them. This
> seems like it'll be a lot of work.

Will this be a lot of work? Maybe :) Here's a patch which implements
this solution. In theory this same approach could be used to make sure
all implicit scope rules are honored.


Teach the Python bindings to keep editors in scope for as long as the objects
which are built from the editor.

Suggested by: glasser

[ In subversion/bindings/swig ]

* python/tests/ra.py
  (test_update): Test lifetime of editors.

* python/libsvn_swig_py/swigutil_py.h, python/libsvn_swig_py/swigutil_py.c
  (svn_swig_py_add_kid_to_pool): New function.

* include/proxy_apr.swg
  (apr_pool_t.__init__, apr_pool_t.clear, apr_pool_t.destroy): Clear kids list.
  (apr_pool_t._add_kid): New function.

 * svn_delta.i
   (py_editor, EDITOR): Update Python typemaps to save references to
the editor in a pool
   so that we can be sure they live long enough.


The test case in this patch is pretty basic. I'd like to see a much
better test case before this gets committed (to test the implications
of this patch on functions other than update), but I don't have a lot
of time to work on this. Do you or epg have time to give this patch
some more rigorous tests?



To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org

Received on 2008-05-17 23:36:07 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.