thread-safe exceptions

It appears to me that the SvnException objects are not
threadsafe. A 
number of them are instantiated within an AllowPythonThreads
scope, and 
its constructor makes extensive use of Python objects. I
believe the 
correct solution is to have the SvnException store the
Subversion error 
object until the exception is thunked into a Python
exception. I've 
included a proposed patch.

You can use the included code to duplicate. I've seen it
cause Python to 
go into a hard loop, crash, or die with a GC error.

-Matthias Miller


""" Sample to illustrate thread-safety issue.
Requires a working copy 
path or URL. """
import pysvn
import sys
import thread
import time

def exceptions(url_or_path):
    while True:
        print thread.get_ident()
        pysvn.Client().status(url_or_path)
        try:
            pysvn.Client().status('This is a path that does
not exist.')
        except pysvn.ClientError, error:
            pass
        else:
            assert False

def main():
    path, = sys.argv[1:]
    for i in range(10):
        thread.start_new_thread(exceptions, (path,))
    while True:
        time.sleep(1)

if __name__ == '__main__':
    main()


------------------------------------------------------------
---------
To unsubscribe, e-mail: users-unsubscribepysvn.tigris.org
For additional commands, e-mail: users-helppysvn.tigris.org

Matthias Miller wrote:
> It appears to me that the SvnException objects are not
threadsafe. A 
> number of them are instantiated within an
AllowPythonThreads scope, 
> and its constructor makes extensive use of Python
objects. I believe 
> the correct solution is to have the SvnException store
the Subversion 
> error object until the exception is thunked into a
Python exception. 
> I've included a proposed patch.

It looks like my previous patch depended on
Microsoft-specific compiler 
behavior. I've included an updated patch.

-Matthias Miller

------------------------------------------------------------
---------
To unsubscribe, e-mail: users-unsubscribepysvn.tigris.org
For additional commands, e-mail: users-helppysvn.tigris.org

On Apr 18, 2008, at 14:09, Matthias Miller wrote:

> It appears to me that the SvnException objects are not
threadsafe.  
> A number of them are instantiated within an
AllowPythonThreads  
> scope, and its constructor makes extensive use of
Python objects. I  
> believe the correct solution is to have the
SvnException store the  
> Subversion error object until the exception is thunked
into a  
> Python exception. I've included a proposed patch.
>
> You can use the included code to duplicate. I've seen
it cause  
> Python to go into a hard loop, crash, or die with a GC
error.

Yes this is not thread safe.

The reason that SvnException copies the error state into
python  
objects in the c'tor is to avoid very nasty problems with
object life  
time
when the pool is destroyed.


Using the allowThisThread() gets the GIL back so that the  
SvnException can do what it wants with Python.

Committed as r933.

Can you review and see if you think it fixes the thread
safety issue  
please?

Barry


------------------------------------------------------------
---------
To unsubscribe, e-mail: users-unsubscribepysvn.tigris.org
For additional commands, e-mail: users-helppysvn.tigris.org

Barry Scott wrote:
>
> On Apr 18, 2008, at 14:09, Matthias Miller wrote:
>
>> It appears to me that the SvnException objects are
not threadsafe. A 
>> number of them are instantiated within an
AllowPythonThreads scope, 
>> and its constructor makes extensive use of Python
objects. I believe 
>> the correct solution is to have the SvnException
store the Subversion 
>> error object until the exception is thunked into a
Python exception. 
>> I've included a proposed patch.
>>
>> You can use the included code to duplicate. I've
seen it cause Python 
>> to go into a hard loop, crash, or die with a GC
error.
>
> Yes this is not thread safe.
>
> The reason that SvnException copies the error state
into python 
> objects in the c'tor is to avoid very nasty problems
with object life 
> time
> when the pool is destroyed.

Gotcha. I missed that.

> Using the allowThisThread() gets the GIL back so that
the SvnException 
> can do what it wants with Python.
>
> Committed as r933.
>
> Can you review and see if you think it fixes the thread
safety issue 
> please?

Yeah, I think that should do the trick. Thanks!

-Matthias Miller

------------------------------------------------------------
---------
To unsubscribe, e-mail: users-unsubscribepysvn.tigris.org
For additional commands, e-mail: users-helppysvn.tigris.org