#110 Deadlock seen when calling DataSources.destroy()

[Phillip Henry]

Our test suite occasionally freezes and invoking jstack on the JVM results in the JVM telling us that there is indeed a deadlock.

It appears that a lock is obtained on the C3P0Registry class in the static markClosed method. The thread with this lock then tries to remove the PooledDataSource from a static HashSet. The process for for removing an item from a HashSet may necessitate calling equals on (potentially all) the items in the Set. Note that equals on this implementation of PooledDataSource calls PoolBackedDataSourceBase.getIdentityToken() which is a synchronised method.

Meanwhile, another thread is closing this instance of PoolBackedDataSource (see the close() method in its superclass AbstractPoolBackedDataSource). This method is also synchronized. Having obtained the the lock on this object, the thread then tries to call the static method C3P0Registry.markClosed. However, since this method it synchronized and static and the first thread holds the lock, it can go no further.

At the same time, if the first thread is calling synchronized methods on the PoolBackedDataSourceBase (via calls to HashSet.remove), the JVM deadlocks.

Suggest that the PoolBackedDataSourceBase.getIdentityToken() method is not synchronized. If this method is synchronized for JMM visibility reasons, then the field should be volatile. However, note that this class is autogenerated.

The salient part of the attached stack dump is:

Found one Java-level deadlock:

"pool-1-thread-9":
waiting to lock monitor 7fd66e042610 (object 7f467fed0, a com.mchange.v2.c3p0.PoolBackedDataSource),
which is held by "pool-1-thread-7"
"pool-1-thread-7":
waiting to lock monitor 7fd67300ca80 (object 7fbbbf838, a java.lang.Class),
which is held by "pool-1-thread-9"

Java stack information for the threads listed above:

"pool-1-thread-9":
at com.mchange.v2.c3p0.impl.PoolBackedDataSourceBase.getIdentityToken(PoolBackedDataSourceBase.java:80)
- waiting to lock <7f467fed0> (a com.mchange.v2.c3p0.PoolBackedDataSource)
at com.mchange.v2.c3p0.impl.AbstractIdentityTokenized.equals(AbstractIdentityTokenized.java:40)
at java.util.HashMap.removeEntryForKey(HashMap.java:556)
at java.util.HashMap.remove(HashMap.java:538)
at java.util.HashSet.remove(HashSet.java:216)
at com.mchange.v2.c3p0.C3P0Registry.markClosed(C3P0Registry.java:258)
- locked <7fbbbf838> (a java.lang.Class for com.mchange.v2.c3p0.C3P0Registry)
at com.mchange.v2.c3p0.impl.AbstractPoolBackedDataSource.close(AbstractPoolBackedDataSource.java:413)
- locked <7f4783030> (a com.mchange.v2.c3p0.PoolBackedDataSource)
at com.mchange.v2.c3p0.impl.AbstractPoolBackedDataSource.close(AbstractPoolBackedDataSource.java:429)
at com.mchange.v2.c3p0.DataSources.destroy(DataSources.java:359)
at com.mchange.v2.c3p0.DataSources.destroy(DataSources.java:335)
.
.
.
"pool-1-thread-7":
at com.mchange.v2.c3p0.C3P0Registry.markClosed(C3P0Registry.java:258)
- waiting to lock <7fbbbf838> (a java.lang.Class for com.mchange.v2.c3p0.C3P0Registry)
at com.mchange.v2.c3p0.impl.AbstractPoolBackedDataSource.close(AbstractPoolBackedDataSource.java:413)
- locked <7f467fed0> (a com.mchange.v2.c3p0.PoolBackedDataSource)
at com.mchange.v2.c3p0.impl.AbstractPoolBackedDataSource.close(AbstractPoolBackedDataSource.java:429)
at com.mchange.v2.c3p0.DataSources.destroy(DataSources.java:359)
at com.mchange.v2.c3p0.DataSources.destroy(DataSources.java:335)

[Phillip Henry]

from calling jstack on our deadlocked test suites

[Phillip Henry]

BTW, we're using 0.9.1.2 but it seems the same in the master branch:
$ git describe --tags
c3p0-0.9.2-pre4-2-gfafb230
$ git status
# On branch master
nothing to commit (working directory clean)

[Steve Waldman]

Phillip,

Thanks for the detailed report. I won't have a chance to take a very detailed look until a bit later, but your point about eliminating synchronization of access on identity token is well taken. volatile would easily do.

Please do let me know what version of c3p0 you are using.

Thanks again.

[Steve Waldman]

Oh, oops! I didn't notice your comment below. Thanks for letting me know the version.. That was all I needed.

[Steve Waldman]

Should be resolved as of c3p0-0.9.2-pre5. Thanks!