wkim / h2database

Automatically exported from code.google.com/p/h2database
0 stars 0 forks source link

Dead Lock when calling a Linked Table in a multi-threaded fashion. #453

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
Please send a question to the H2 Google Group or StackOverflow first,
and only then, once you are completely sure it is an issue, submit it here.
The reason is that only very few people actively monitor the issue tracker.

Before submitting a bug, please also check the FAQ:
http://www.h2database.com/html/faq.html

What steps will reproduce the problem?
(simple SQL scripts or simple standalone applications are preferred)

It's hard to list steps to reproduce this problem, because it is a deadlock.  
However, any situation where multiple threads are using a Linked Table at the 
same time has the potential to create this dead lock.

What is the expected output? What do you see instead?

The expected result is that threads calling into H2 do not lock on each other.  
Instead, we are seeing threads lock on one another intermittently.

What version of the product are you using? On what operating system, file
system, and virtual machine?

We are using version 168 of H2, although this problem still exists in the 
latest version.  We are using Ubuntu 10.04.1 LTS.

Do you know a workaround?

No.

What is your use case, meaning why do you need this feature?

Our use case is being able to use a linked table in H2, and being able to call 
it from multiple threads.

How important/urgent is the problem for you?

This is not a particularly urgent problem for us, since it happens 
intermittently.

Please provide any additional information below.

We are experiencing occasional dead locks when using linked tables.  By taking 
stack traces of the deadlocked JVM, we've found a dead lock situation in 
LinkedTableConnection.java.  The dead lock occurs when one thread is calling 
the static method TableLinkConnection.open() at the same time that another 
thread is calling the instance method TableLinkConnection.close().  

If you check out the code, the static open() method synchronizes on the map of 
TableLinkConnection instances ("map"), then finds a shared TableLinkConnection 
instance in the map, and synchronizes on that in order to increment its 
reference count ("useCounter").  The instance method close() is itself 
synchronized, which locks on the TableLockConnection instance that close() is 
being called for.  This method decrements the reference count ("userCounter") 
and then locks on the shared static map of TableLinkConnnection instances 
("map") in order to remove itself.

So.  If the two methods are called at the same time, and the thread calling the 
open() method has locked the map while the thread calling the close() method 
has (implicitly) locked the TableLinkConnection instance, then each thread will 
proceed further, and attempt to lock the resource that the other method is 
locking - the instance of the TableLinkConnection in the open() thread case, 
and the map of TableLinkConnection instances in the close() case.  However, 
since the other thread all ready has that desired resource locked, each thread 
will begin waiting on the *other* thread to release its resource lock, and 
neither thread will continue to execute.

I have modified this class slightly in order to remove the synchronization used 
to protect the reference count "useCounter", and instead use an AtomicInteger 
to hold the reference count.  This should allow the open() method to continue 
it operation and release it's synchronization lock on "map", and everything 
should be happy.

Note also that I removed the synchronized block on the TableLinkConnection 
instance in the static open() method for the call to t.open().  This 
synchronization would seem to be unneeded, since we only just created this 
TableLinkConnection instance, and have not shared the reference to it with 
anyone else, since we have not yet inserted into the map.

I have kept the synchronized keyword on the close() method, although I think it 
could probably be removed at this point.  My guess is that it was synchronized 
in order to protect the useCounter during decrement, but I am concerned about 
the JdbcUtils.closeSilently() call, and so left the synchronized keyword there.

Original issue reported on code.google.com by tom.even...@yieldex.com on 27 Mar 2013 at 4:47

Attachments:

GoogleCodeExporter commented 8 years ago
Fixed in revision 4716. 
Thanks for your patch.
I did not use your code, because there was another race condition, so I just 
simplified the existing locking.

Original comment by noelgrandin on 2 Apr 2013 at 7:42