You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Jelmer Vernooij <je...@samba.org> on 2008/04/13 00:11:20 UTC
[PATCH/RFC] Fix refcount on some Python callbacks
The attached patch fixes the reference counting in the Python part of %
callback_typemap. At the moment the reference counter of the Python
function is not increased when passing it as a baton. This means that if
Subversion is the only one still referencing the function it will be
garbage collected, causing Subversion to segfault when it tries to call
the Python callback.
However, this patch doesn't decrement the reference counter once the
callback is no longer used since there is no place to free batons as far
as I know. That means we'll potentially keep Python methods around in
memory that aren't used. Is that a problem or is there perhaps some way
to work around this?
Cheers,
Jelmer
--
Jelmer Vernooij <je...@samba.org> - http://samba.org/~jelmer/
Re: [PATCH/RFC] Fix refcount on some Python callbacks
Posted by Eric Gillespie <ep...@pretzelnet.org>.
Jelmer Vernooij <je...@samba.org> writes:
> Can you give an example of how you do this? I tried adding them to a
> list but that didn't seem to work.
See http://gvn.googlecode.com/svn/trunk/lib/gvn/svnauth.py
# The bindings don't hold references to my callbacks, so when we
# return, they're all freed. Work around it for now.
return (svn_config_dir_utf8, SimplePrompt, UsernamePrompt, SSLServerTrustPrompt, SSLClientCertPrompt, SSLClientCertPwPrompt, auth_baton)
--
Eric Gillespie <*> epg@pretzelnet.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH/RFC] Fix refcount on some Python callbacks
Posted by Jelmer Vernooij <je...@samba.org>.
On Sat, 2008-04-12 at 18:44 -0700, Eric Gillespie wrote:
> Jelmer Vernooij <je...@samba.org> writes:
>
> > callback_typemap. At the moment the reference counter of the Python
> > function is not increased when passing it as a baton. This means
> that if
> > Subversion is the only one still referencing the function it will be
> > garbage collected, causing Subversion to segfault when it tries to
> call
> > the Python callback.
> I work around this bug in gvn by holding onto extra copies of the
> callback functions myself.
Can you give an example of how you do this? I tried adding them to a
list but that didn't seem to work.
Cheers,
Jelmer
--
Jelmer Vernooij <je...@samba.org> - http://samba.org/~jelmer/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH/RFC] Fix refcount on some Python callbacks
Posted by Eric Gillespie <ep...@pretzelnet.org>.
Jelmer Vernooij <je...@samba.org> writes:
> callback_typemap. At the moment the reference counter of the Python
> function is not increased when passing it as a baton. This means that if
> Subversion is the only one still referencing the function it will be
> garbage collected, causing Subversion to segfault when it tries to call
> the Python callback.
I work around this bug in gvn by holding onto extra copies of the
callback functions myself.
> However, this patch doesn't decrement the reference counter once the
> callback is no longer used since there is no place to free batons as far
> as I know. That means we'll potentially keep Python methods around in
> memory that aren't used. Is that a problem or is there perhaps some way
> to work around this?
That is a problem. Hopefully you or someone else can find the
solution...
--
Eric Gillespie <*> epg@pretzelnet.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: [PATCH/RFC] Fix refcount on some Python callbacks
Posted by "Bert Huijben (TCG)" <b....@competence.biz>.
> -----Original Message-----
> From: Jelmer Vernooij [mailto:jelmer@samba.org]
> Sent: zondag 13 april 2008 2:11
> To: dev@subversion.tigris.org
> Subject: [PATCH/RFC] Fix refcount on some Python callbacks
>
> However, this patch doesn't decrement the reference counter once the
> callback is no longer used since there is no place to free batons as
> far
> as I know. That means we'll potentially keep Python methods around in
> memory that aren't used. Is that a problem or is there perhaps some way
> to work around this?
(I faced a few of the same issues while designing the SharpSvn library)
In some cases it's an option to do the cleanup from an apr pool cleanup function.
I use this for marshalling managed exception instances from the callback handlers back to the caller of the function.
(.NET uses a mark and sweep garbage collector; so I must keep a reference alive).
As all errors have their own root apr pool, their pools are always cleared when svn_error_clear() is called; this makes it possible for the subversion library to clear an exception triggered by managed code.
In SharpSvn callbacks passed to subversion functions are kept alive by the functions calling the subversion functions; so they don't need special treatment. (And they call back into the managed code via a weak reference. This always succeeds as the calling function also has a reference)
(Using the apr pools in a try finally construction around all calls cleans up automatically on almost all thrown exceptions)
Bert Huijben
>
> Cheers,
>
> Jelmer
>
> --
> Jelmer Vernooij <je...@samba.org> - http://samba.org/~jelmer/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH/RFC] Fix refcount on some Python callbacks
Posted by Martin von Gagern <Ma...@gmx.net>.
David Glasser wrote:
> David James posted a patch to help with something similar (editors,
> not callbacks):
Comparing my approach to his, it looks to me like he would add
references to the pools while I would have added those references to
python objects. I guess the former more closely resembles the native
workings of C with APR, while the latter is more in the Python style.
> I didn't really feel qualified to know if it was the right solution
> though, and ended up dropping it.
I'm a newcomer to Python, have never really worked with APR, so I
wouldn't be "qualified" in any way either. On the other hand, I would
assume that most attempts at a fix, even if not in the best of styles,
would be better than the segmentation faults that occur now.
Greetings,
Martin
Re: [PATCH/RFC] Fix refcount on some Python callbacks
Posted by David Glasser <gl...@davidglasser.net>.
On Mon, Jun 9, 2008 at 11:53 AM, Martin von Gagern
<Ma...@gmx.net> wrote:
> Jelmer Vernooij wrote:
>>
>> The attached patch fixes the reference counting in the Python part of %
>> callback_typemap. At the moment the reference counter of the Python
>> function is not increased when passing it as a baton. This means that if
>> Subversion is the only one still referencing the function it will be
>> garbage collected, causing Subversion to segfault when it tries to call
>> the Python callback.
>> However, this patch doesn't decrement the reference counter once the
>> callback is no longer used since there is no place to free batons as far
>> as I know. That means we'll potentially keep Python methods around in
>> memory that aren't used. Is that a problem or is there perhaps some way
>> to work around this?
>
> Hi!
>
> I'd like to warm up this old thread again, as I have a new suggestion.
> Generally you have some python function, pass it to some interface code that
> builds a wrapper around it to turn it into a c callback type, and wraps that
> internal c thing back into a python object. Correct so far?
>
> Now the only thing needed in order to get clean garbage collection would be
> to attach the python function to the generated python object on the python
> level. I've got a demo for this in one instance:
>
> provider = svn_auth_get_ssl_server_trust_prompt_provider(callback)
> provider.callback_reference = callback
>
> So the subversion bindings see provider as the swig wrapper and can use that
> to access the wrapped callback, just as before. Python, on the other hand,
> sees that the object provider holds a reference to the function callback, so
> it can't garbage collect the latter as long as the provider is live.
>
> This is a simple example and executed in one bzr-svn branch:
> https://code.launchpad.net/~gagern/bzr-svn/bug238529
> related to https://bugs.launchpad.net/bugs/238529
>
> I'm no proficient enough in using swig to formulate this yet, but I would
> assume that this kind of approach should work for all cases of functions
> that take one python object and return another python object that internally
> references the former but doesn't do so in python yet.
>
> So instead of addressing this in each and every application using the python
> bindings, it would be preferable to solve this on the subversion side. Do
> you think this can be done? Is someone willing to do so? Or if not, maybe
> willing to point me or others into the right direction?
This is definitely a real problem.
David James posted a patch to help with something similar (editors,
not callbacks):
http://svn.haxx.se/dev/archive-2008-05/0738.shtml
I didn't really feel qualified to know if it was the right solution
though, and ended up dropping it.
--dave
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH/RFC] Fix refcount on some Python callbacks
Posted by Martin von Gagern <Ma...@gmx.net>.
Jelmer Vernooij wrote:
> The attached patch fixes the reference counting in the Python part of %
> callback_typemap. At the moment the reference counter of the Python
> function is not increased when passing it as a baton. This means that if
> Subversion is the only one still referencing the function it will be
> garbage collected, causing Subversion to segfault when it tries to call
> the Python callback.
>
> However, this patch doesn't decrement the reference counter once the
> callback is no longer used since there is no place to free batons as far
> as I know. That means we'll potentially keep Python methods around in
> memory that aren't used. Is that a problem or is there perhaps some way
> to work around this?
Hi!
I'd like to warm up this old thread again, as I have a new suggestion.
Generally you have some python function, pass it to some interface code
that builds a wrapper around it to turn it into a c callback type, and
wraps that internal c thing back into a python object. Correct so far?
Now the only thing needed in order to get clean garbage collection would
be to attach the python function to the generated python object on the
python level. I've got a demo for this in one instance:
provider = svn_auth_get_ssl_server_trust_prompt_provider(callback)
provider.callback_reference = callback
So the subversion bindings see provider as the swig wrapper and can use
that to access the wrapped callback, just as before. Python, on the
other hand, sees that the object provider holds a reference to the
function callback, so it can't garbage collect the latter as long as the
provider is live.
This is a simple example and executed in one bzr-svn branch:
https://code.launchpad.net/~gagern/bzr-svn/bug238529
related to https://bugs.launchpad.net/bugs/238529
I'm no proficient enough in using swig to formulate this yet, but I
would assume that this kind of approach should work for all cases of
functions that take one python object and return another python object
that internally references the former but doesn't do so in python yet.
So instead of addressing this in each and every application using the
python bindings, it would be preferable to solve this on the subversion
side. Do you think this can be done? Is someone willing to do so? Or if
not, maybe willing to point me or others into the right direction?
Greetings,
Martin von Gagern