code review request: 7118809: rcache deadlock

[Weijun Wang]

Hi Valerie

Please review my fix:

    http://cr.openjdk.java.net/~weijun/7118809/webrev.00/

Basically, CacheTable.put() operates on ReplayCache inside it, and 
ReplayCache.put() operates on CacheTable containing it, and both methods 
are synchronized and using thread-safe Hashtable.

My solution is to move the "table.remove(principal)" line in 
ReplayCache.put() outside the method. I search thru JDK and 
CacheTable.put() is the only place the method is called:

     public synchronized void put(String principal, AuthTime time, long 
currTime) {
         ReplayCache rc = super.get(principal);
         if (rc == null) {
             if (DEBUG) {
                 System.out.println("replay cache for " + principal + " 
is null.");
             }
             rc = new ReplayCache(principal, this);
             rc.put(time, currTime);
             super.put(principal, rc);
         }
         else {
             rc.put(time, currTime);
             // re-insert the entry, since rc.put could have removed the 
entry
             super.put(principal, rc);
             if (DEBUG) {
                 System.out.println("replay cache found.");
             }
         }
     }

Curiously, you can see after each call, the ReplayCache object is added 
back into the CacheTable. Therefore, the remove action is useless.

Maybe the most correct way is to remove a ReplayCache from a CacheTable 
if it's empty, but that re-insert line was added in a security fix some 
years ago which I cannot decipher.

So my fix simply removes the "remove" call in ReplayCache.

No regression test, hard to reproduce failure.

Thanks
Max

-------- Original Message --------
*Change Request ID*: 7118809
*Synopsis*: rcache deadlock

=== *Description* 
============================================================
A DESCRIPTION OF THE PROBLEM :
The program as below:
--------------------------------------------------
import sun.security.krb5.internal.rcache.*;
import sun.security.krb5.internal.*;
import java.util.*;

public class KTest2 {
   public static void main(String [] a) throws Exception{
     final CacheTable ct = new CacheTable();
     final long time = System.currentTimeMillis();
     ct.put("TheOnlyOne", new AuthTime( time - 
Krb5.DEFAULT_ALLOWABLE_CLOCKSKEW * 1000L, 0), time);
     final ReplayCache rc = (ReplayCache) ct.get("TheOnlyOne");
     new Thread() {
       public void run() {
         rc.put(new AuthTime( time - Krb5.DEFAULT_ALLOWABLE_CLOCKSKEW * 
1000L, 0), time + 300*1000);
       }
     }.start();
     ct.put("TheOnlyOne", new AuthTime( time - 
Krb5.DEFAULT_ALLOWABLE_CLOCKSKEW * 1000L, 0), time);
   }
}
--------------------------------------------------
When compiles as in: javac KTest2.java
and executed as in: java KTest2
can deadlock the hosting JVM as is reproduced by the stack-trace dump, 
below:
-------------------------------------------------
Found one Java-level deadlock:
=============================
"Thread-0":
   waiting to lock monitor 0x0921d720 (object 0xa5621b80, a 
sun.security.krb5.internal.rcache.CacheTable),
   which is held by "main"
"main":
   waiting to lock monitor 0x0921caa0 (object 0xa5622fb8, a 
sun.security.krb5.internal.rcache.ReplayCache),
   which is held by "Thread-0"

Java stack information for the threads listed above:
===================================================
"Thread-0":
	at java.util.Hashtable.remove(Hashtable.java:435)
	- waiting to lock <0xa5621b80> (a 
sun.security.krb5.internal.rcache.CacheTable)
	at sun.security.krb5.internal.rcache.ReplayCache.put(ReplayCache.java:123)
	- locked <0xa5622fb8> (a sun.security.krb5.internal.rcache.ReplayCache)
	at KTest2$1.run(KTest2.java:13)
"main":
	at sun.security.krb5.internal.rcache.ReplayCache.put(ReplayCache.java:62)
	- waiting to lock <0xa5622fb8> (a 
sun.security.krb5.internal.rcache.ReplayCache)
	at sun.security.krb5.internal.rcache.CacheTable.put(CacheTable.java:59)
	- locked <0xa5621b80> (a sun.security.krb5.internal.rcache.CacheTable)
	at KTest2.main(KTest2.java:16)

Found 1 deadlock.
...