Issue898709

These modifications were made to synchronize some of 
the initialization methods on an object lock and prevent 
multiple threads from accessing data simultaneously.  
Some problems were found that some of the member 
variables were being simultaneously read and written by 
different threads, so some extra precautions were added 
in to the code to verify that access to these variables is 
thread safe.  

Attached is a file containing diff's of the files modified:
org.python.core.PyFile
org.python.core.PyJavaClass
org.python.core.StdoutWrapper

Also, a complete copy of the source with the 
recommended modifications is contained in the attached 
file.  

Logged In: YES 
user_id=61408

can you submit some kind of minimal test reproducing the
problems you  were having. Especially wrt to PyJavaClass.

Thanks.

Logged In: YES 
user_id=971697

I'm not sure if this is the right way to respond to your reply, 
but nevertheless, here goes.

The problems described in this ticket were found by using the 
JProbe Threadalyzer hooked up to the jvm containing multiple 
jython interpreters.  When running multiple intrepreters in the 
same JVM, they would often compete for access to the same 
data members when compiling and executing scripts.  

Logged In: YES 
user_id=971697

If it would help, I can prepare and attach a document 
containing the stack traces showing for the threads trying to 
read and write the data simultaneously.

Logged In: YES 
user_id=61408

yes, that would be useful, or at least a listing of the member
involved.

Thanks.

Logged In: YES 
user_id=971697

I have attached another document that will show the stack 
traces and member variables involved in the data races.  It 
contains screen shots taken from JProbe, which is why it is 
such a large file.  The stack traces show the order of 
execution for both the reader and the writer that is 
attempting to access the same data member.  

Logged In: YES 
user_id=971697

I have attached another document that will show the stack 
traces and member variables involved in the data races.  It 
contains screen shots taken from JProbe, which is why it is 
such a large file.  The stack traces show the order of 
execution for both the reader and the writer that is 
attempting to access the same data member.  

Logged In: YES 
user_id=971697

I have attached another document that will show the stack 
traces and member variables involved in the data races.  It 
contains screen shots taken from JProbe, which is why it is 
such a large file.  The stack traces show the order of 
execution for both the reader and the writer that is 
attempting to access the same data member.  

Logged In: YES 
user_id=971697

Well, the original file with the screenshots was too large, so I 
typed in the stack traces to removed the pictures.  I 
apologize for any typos.  

Logged In: YES 
user_id=61408

first, no problem, and thanks. It is not that didn't believe
there was a problem, but I like to know what exactly I'm fixing.
So far, I didn't notice it but yes PyJavaClass is abusing in
some spots
the "Double-Checked Locking" idiom which is not safe either
under the old or new Java memory model. I think they are
there from long time in the history of the project.

1) I fear that PyFile.softspace problem is just the tip of
an iceberg,
I think our file layer is in general not thread-safe, but
yes at least print statements should be, I think the
synchronization should be done some level higher for that.

2) the analysis is dynamic right? and there are also some
false positives e.g
    private static int nameindex=0;
    public static synchronized String getName() {
        String name = "org.python.pycode._pyx"+nameindex;
        nameindex += 1;
        return name;
    }

3) yes there are data races for initialized,
constructorsInitialized ,__bases__,proxyClass and __init__
in PyJavaClass and also trashing of missingAttributes and
keywords.
 that  were not spotted.

4) why do you exactly decided to go with a class-level lock,
is there a particular reason or just defensive programming?

I understand that in the scheme using both a class lock and
an instance lock, there is a risk to try to acquire them in
different orders on different threads with the danger of
deadlocking, OTOH as far as I see, the code working under
the old class lock
doesn't invoke synchronized code on single instances.

All fields should be set by each single PyJavaClass on its own,
except for proxyClass which should be initalized holding a
class-level lock because the Class->PyJavaClass table should
be manipulated at the same time.

5) is this for 2.1 code or the CVS?

Logged In: YES 
user_id=971697

The analysis provided is a dynamic one.  Therefore, it only 
shows those problems that it actually encounters.  Thus 
there may be additional problems that have not yet been 
uncovered.  

I believe that it must provide false positives, because of the 
example you presented (in #2).  I could not figure out how 
nameindex could be causing a problem in that situation!

The reason I used a class level lock was mainly defensive 
programming.  I initially tried providing a separate lock for 
each of the member variables, but this led to very 
complicated locking schemes due to the fact that the 
variables are used in a lot of different methods.  Not knowing 
enough about how the methods interact with each other, I 
generally ended up causing deadlocks in the PyJavaClass.  So 
I went back to a class lock.  

Your suggestion of locking the proxyClass may work.  I do not 
want to claim that my solution is the only one.  It is just what 
I was able to come up with. 

Finally, the code is from the 2.1 version, and not the latest 
version from CVS.  I checked the CVS to see if anything had 
been modified in the PyJavaClass that may affect the 
threading because if it had already been fixed, I did not want 
to fix it again.  But I couldn't find any fixes that dealt with 
the threading problems I saw.  

Logged In: YES 
user_id=61408

this is need to be address in the polishing of the
*Class/*Instance but indeed simplifying what goes on with
lazy filling of __dict__ etc which we should keep and lazy
loaded classes which can forgo is the way to go but heavely
using class level locks is not the right approach.

PyFile and StdoutWrapper have completely changed, so this is too far gone to apply.