[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?

Cheers,

David

---------------------------------------------------------------------
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.