I'd like to think that transfer() is only called when adding / removing, so once we get into a 'ready state' the synchronization should be unnecessary. We do a lot of lazy loading though so getting into that state might be tricky to predict.
I'd say go ahead and add the synchronization in 1.1.x. -mike On Tue, Mar 29, 2011 at 9:49 PM, Ravi Palacherla <[email protected] > wrote: > The java source code might have been corrected in jdk6. > I may not need to synchronize this in trunk but need to change 1.1.x > branch. > I will spend more time and update this thread. > > Regards, > Ravi. > > On Mar 29, 2011, at 8:07 PM, Ravi Palacherla wrote: > > > Hi Mike, > > > > As the java doc recommends that structural modification of > HashMap/TreeMap should be done only with proper synchronization. > > The bug was not considered to be java bug but a solution to synchronize > the HashMap is recommended. > > > > Hence I think we should synchronize the TreeMap in Schema.java to avoid > the infinite loop in TreeMap.get. > > > > Here is a blog that explains the cause of this issue: > > http://lightbody.net/blog/2005/07/hashmapget_can_cause_an_infini.html > > > > Regards, > > Ravi. > > > > On Mar 28, 2011, at 12:52 PM, Michael Dick wrote: > > > >> Hi Ravi, > >> > >> It'd be nice to know which levels of Java it affects (I'm assuming the > bug > >> is fixed in some level). If it's not planned for Java 6 then we probably > >> need to get the fix in. > >> > >> -mike > >> > >> On Mon, Mar 28, 2011 at 1:34 PM, Ravi Palacherla < > [email protected] > >>> wrote: > >> > >>> Hi Mike, > >>> > >>> The link used to work till yesterday. I will see if I can find another > link > >>> that shows the issue. > >>> > >>> It is not a sun bug but using an un-synchronized hashMap is resulting > in > >>> the hashmap to get corrupted and it is spinning for ever at hashmap.get > >>> code. > >>> 6423457 (coll) High cpu usage in HashMap.get() > >>> > >>> This has been tracked as sun bug# 6423457 and the solution provided is > to > >>> synchronize the hashMap. > >>> Even though the issue might have been triggered because of structural > >>> modification of HashMap outside a synchronized block the thread dump > shows > >>> that the thread is stuck at HashMap.get. > >>> > >>> The same customer had a similar issue previously with > TableJDBCSeq.java: > >>> > >>> java.lang.Thread.State: RUNNABLE > >>> @ at java.util.HashMap.__tc_get(Unknown Source) > >>> @ at java.util.HashMap.get(Unknown Source) > >>> @ at > >>> @ > >>> > org.apache.openjpa.jdbc.kernel.TableJDBCSeq.getStatus(TableJDBCSeq.java:308) > >>> @ at > >>> @ > >>> > org.apache.openjpa.jdbc.kernel.TableJDBCSeq.nextInternal(TableJDBCSeq.java:25 > >>> @ 3) > >>> > >>> This has been fixed by using concurrentHashMap as part of OPENJPA-1765. > >>> > >>> > >>> I think the issue is similar but with TreeMap in openJPA's schema.java > >>> code. > >>> > >>> Here is a test code that I used to replicate the issue, but > unsuccessful so > >>> far: > >>> > >>> public void testSchemaTable() throws InterruptedException { > >>> final Schema _schema = new SchemaGroup().addSchema("schema"); > >>> > >>> for (int i=0;i<LOOPCOUNT;i++) { > >>> _schema.addTable("Table:"+i); > >>> count++; > >>> } > >>> > >>> Thread[] threads = new Thread[THREADS]; > >>> > >>> for(int numThread=0; numThread< THREADS; numThread++) { > >>> if( numThread % (THREADS-10) == 0) { > >>> threads[numThread] = new Thread(new Runnable() { > >>> public void run() { > >>> int lc = LOOPCOUNT+count; > >>> for (int i=count;i<lc;i++,count++) { > >>> _schema.addTable("Table:"+i); > >>> } > >>> } > >>> }); > >>> } else { > >>> threads[numThread] = new Thread(new Runnable() { > >>> public void run() { > >>> int lc = LOOPCOUNT+count; > >>> for (int i=0;i<LOOPCOUNT;i++) { > >>> _schema.getTable("Table:"+i); > >>> } > >>> } > >>> }); > >>> } > >>> > >>> } > >>> > >>> for (int threadCount=0; threadCount<THREADS;threadCount++) { > >>> threads[threadCount].start(); > >>> } > >>> > >>> for (int threadCount=0; threadCount<THREADS;threadCount++) { > >>> threads[threadCount].join(); > >>> } > >>> } > >>> > >>> > >>> Regards, > >>> Ravi. > >>> > >>> On Mar 28, 2011, at 11:00 AM, Michael Dick wrote: > >>> > >>>> Hi Ravi, > >>>> > >>>> The link didn't work for me, is this a bug in the TreeMap > implementation? > >>>> > >>>> Multithreaded reads of a TreeMap should be fine. If a thread is adding > or > >>>> deleting a node then we'll need to synchronize - is that what's > happening > >>> in > >>>> your case? > >>>> > >>>> I've cross posted to dev - might get a better audience there. > >>>> > >>>> -mike > >>>> > >>>> On Sun, Mar 27, 2011 at 10:06 PM, Ravi P Palacherla < > >>>> [email protected]> wrote: > >>>> > >>>>> Hi , > >>>>> > >>>>> I see following stuck threads in openJPA code : > >>>>> > >>>>> java.lang.Thread.State: RUNNABLE > >>>>> at java.util.TreeMap.getEntry(TreeMap.java:328) > >>>>> at java.util.TreeMap.get(TreeMap.java:255) > >>>>> at org.apache.openjpa.jdbc.schema.Schema.getTable(Schema.java:115) > >>>>> > >>>>> I think the root cause of the issue is because of unsynchronized > >>> TreeMap. > >>>>> > >>>>> There is a sun issue that explains the same about a hashMap and I > think > >>>>> this > >>>>> is also because of the same issue explained in: > >>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6423457 > >>>>> > >>>>> So, the fix is to synchronize the TreeMap but I do not have a > replicable > >>>>> test case. > >>>>> > >>>>> I created a simple testcase that will create around 100000 treeMap > >>> entries > >>>>> and then around 50 threads trying to access this treeMap but unable > to > >>>>> replicate the issue. > >>>>> > >>>>> Please suggest what you think should be done. > >>>>> Can I just submit the code fix ( synchronizing the treemap) and the > test > >>>>> case ( even though unable to replicate ) ? > >>>>> > >>>>> Regards, > >>>>> Ravi. > >>>>> > >>>>> -- > >>>>> View this message in context: > >>>>> > >>> > http://openjpa.208410.n2.nabble.com/Stuck-threads-at-Schema-getTable-tp6213602p6213602.html > >>>>> Sent from the OpenJPA Users mailing list archive at Nabble.com. > >>>>> > >>> > >>> > > > >
