[Jython-bugs] [ jython-Patches-1846247 ] Patch for [ 1721204 ] there is no threading.local()

[SourceForge.net <no...@so...>]

Patches item #1846247, was opened at 2007-12-07 10:08
Message generated for change (Comment added) made by cgroves
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=312867&aid=1846247&group_id=12867

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
>Status: Closed
>Resolution: Accepted
Priority: 5
Private: No
Submitted By: Mehendran (mehendran)
Assigned to: Nobody/Anonymous (nobody)
Summary: Patch for [ 1721204 ] there is no threading.local()

Initial Comment:
I have implemented thread-local support on jython. 
I have included the py based thread-local
implementation also as in the link 
http://wiki.python.org/jython/ThreadLocalVariables.

I have written a testcase file(test_thread_local.py)
based on the test_threading_local.py for testing the
java based thread-local implementation.

Test files:
1) test_thread_local.py
2) test_threading_local.py

The second test case will be failing because the string
representation of thread-local object is not as expected.
And DocTestSuite.addSuite is expecting only one argument. So I commented the two arguments in 
test_threading_local.py in the calling place of
addSuite method.

Actually _threading_local.py module is attached with
the doctest and that declares MyLocal, subclass of
local type. There will be a test case expecting the 
string "AttributeError: 'MyLocal' object has no 
attribute 'color'". But jython gives
<module_name>.MyLocal in place of MyLocal. 
<module_name> = __main__ or
              =  the file name where the class is
                 declared.
This doctest is used by test_threading_local.py

We may avoid this error by overriding the safeRepr in
PyLocal.java. For py based implementation, we have to step into PyObject.java. I dont think avoiding error is
correct. It might have been implemented for some
purpose. The another way is changing the test case.

Please give me your valuable suggestions/comments.




                















----------------------------------------------------------------------

>Comment By: Charles Groves (cgroves)
Date: 2007-12-12 04:41

Message:
Logged In: YES 
user_id=1174327
Originator: NO

Ahh, I see that threading.py is from CPythonLib now.  Dunno how I got
confused there.  Like you said, I'm only changing it myself because I'd
pretty much already done it trying to understand what was going on, so I
didn't see any point in asking you to do the same thing.

In any case, I applied the patch in r3801.  Thanks!

----------------------------------------------------------------------

Comment By: Mehendran (mehendran)
Date: 2007-12-10 03:37

Message:
Logged In: YES 
user_id=1831942
Originator: YES

Thanks. As you said, I ran your test with the old patch.
The second call is overwriting the existing local. You
have solved that. Good!

I used the same test cases from _threading_local.py as 
I mentioned in the comments.

I saw that threading.py of CPython2.4.4 uses deque. But
I didn't copy the threading.py from any version of CPython.
It was already in CPython/Lib folder. I just inlcuded the 
necessary code to run the thread local. I thinks it is 
from http://svn.python.org/projects/python/branches/release23-maint/Lib.

I thought that there is not much change in the following files.
CPythonLib/threading.py
CPythonLib/_threading_local.py
CPythonLib/test/test_threading_local.py
That's why I copied to CPython/Lib. I will follow your comments 
from now on.
I don't know why you changed the code. You could have suggested/
commented that and I would have done those. Maybe, you thought to
avoid unnecessary thread mailing. That's fine.











----------------------------------------------------------------------

Comment By: Charles Groves (cgroves)
Date: 2007-12-08 19:58

Message:
Logged In: YES 
user_id=1174327
Originator: NO

Because ThreadLocal tdict is static in PyLocal, making a second
threading.local object in a single thread destroys the dict from the first
local object.  Run the following code with your patch and then with CPython
to see what I mean:

import threading
first = threading.local()
first.x = 7
print "After creation:", first.x
second = threading.local()
print "After creating a second:", first.x

I've attached a version of the patch that fixes that, and cleans up the
logic in local_init a bit.  The way you'd written it, an initless subclass
of a subclass of local that had an init would be unable to take args or
kwargs even though it has a superclass that can reasonably handle them.  

I also added tests for that sort of sublclassing and the use of two locals
in a single thread to your test_thread_local.py.  I think we can wait to
bring _threading_local.py over to our Lib whenever we fix that module
naming problem.  It seems like your test does everything in the doctest,
and there's no need for _threading_local as a backup since we're always
going to include PyLocal.  It might be worth filing a bug for the doctest
problem though.

The only thing that's keeping me from committing my version is that I'm
not sure where the version of thread.py you supplied came from.  What
version of CPython was that in?  It's not 2.5: that version uses deque. As
you well know, Jython doesn't have an implementation of deque quite yet :) 
It probably will soon though...

One last thing: please add the .py files you want to add to Lib like I did
in my patch unless they're identical to the ones in CPythonLib.  In that
case, add them to CPythonLib.includes as you did for _threading_local.py 
It's easier for me to figure out what merging in I need to do if they land
in Lib.
File Added: thread_local_groves.diff

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=312867&aid=1846247&group_id=12867