[#GROOVY-1890] MemoryAwareConcurrentReadMap cause thred dead lock,it will hang our application at some time.

groovy

MemoryAwareConcurrentReadMap cause thred dead lock,it will hang our application at some time.

Created: 12/May/07 04:18 PM Updated: 02/Jul/07 10:48 AM

Component/s:

groovy-jdk

Affects Version/s:

1.1-beta-1

Fix Version/s:

1.1-beta-2

Time Tracking:

Not Specified

File Attachments:

MemeoryAwareTest.java (3 kB)
2.

MemeoryAwareTest.java (3 kB)
3.

MemoryAwareConcurrentReadMap.patch (1 kB)

Environment:

Windows Xp /jdk1.5

Issue Links:

Duplicate

This issue is duplicated by:

~~GROOVY-1964~~ deadlock in MemoryAwareConcurrentReadMap.get

Testcase included:

yes

Description

« Hide

The class org.codehaus.groovy.runtime.metaclass.MemoryAwareConcurrentReadMap deal with the realization of the visit with multithreading is not doing well enough, often lead to deadlock. The worst result is that an application will hang because of it.

The problem is the writeLock and writeQueue. I don't think the writeQueue is must be used in the code,and It somewhat redundant.
It's possible to implement the same function without writeQueue.

May be I'm wrong about wrietQueue,but the bug is true and is very Serious .

I can not imagine, if the bug is real, use grail to realize a project. What we will get?

The test case will show the "dead lock" when you run it some times.
I wish the patch should be helpful also.

Thanks.

All

Comments

Work Log

Change History

Sort Order:

[ Permalink | « Hide ]

agilewang added a comment - 12/May/07 04:26 PM

I'm sorry,the test file is not

[ Permalink | « Hide ]

agilewang added a comment - 12/May/07 04:30 PM

I'm sorry,the previous test file used my own MemoryAwareConcurrentReadMap.
This test file is the real test case.It has to be run with jdk1.5 because of the ThreadMXBean.

[ Permalink | « Hide ]

Jochen Theodorou added a comment - 16/May/07 03:03 PM

I applied changes to the class according to your ideas... only a little different... anyway, it would be nice if you could test that. Just downloading the class compiling and exchanging it with the version form the beta should do the job. Or you do a full build of course

[ Permalink | « Hide ]

agilewang added a comment - 17/May/07 06:08 AM

Great,I have downloaded the new source from SVN,and rerun the test case which passed well.

But I have a problem about the method :

private void unlockWrite(){
synchronized (writeLock) { concurrentReads--; if (concurrentReads==0) writeLock.notify(); }
}

Why do you use the writeLock.notify() rather than the writeLock.notifyAll()?

Though the test is passed,I have a little worry about notiy() .

[ Permalink | « Hide ]

Jochen Theodorou added a comment - 17/May/07 06:34 AM

ups, you are right... small mistake. First I thought that would result in multiple concurrent writes, but as wait needs to reacquire the monitor first that will not happen.

But I noticed the implementation does still have a whole... If I have two read threads, where the first (T1) is running in unlockWrite, the second (T2) waiting in lockWrite and the write thread (T3) waiting in waitForWriteState. If now T3 starts before T2 all is ok, if T2 starts before T3 I have a concurrent write and read... which means I need to add the loop again, because if T2 starts first concurrentRead is!=0 and the write waits for the next time... ok so two false cahnges, fixed in a few minutes

[ Permalink | « Hide ]

agilewang added a comment - 17/May/07 10:35 AM

I'll update to your newest implements.

Other problem is serious too, please see here http://jira.codehaus.org/browse/GROOVY-1882.

I don't know if you have viewed it,I wish you should take some time to check the report.
Some application may not be stable ought to it if it's a real bug ,

Thank you