7056731: Race condition in CORBA code causes re-use of ABORTed connections
authorcoffeys
Thu, 16 Aug 2012 10:33:01 +0100
changeset 13619 187e43fd5607
parent 13445 b67041a6cb50
child 13620 280c762f0ce5
7056731: Race condition in CORBA code causes re-use of ABORTed connections Reviewed-by: lancea Contributed-by: d.macdonald@auckland.ac.nz
corba/src/share/classes/com/sun/corba/se/impl/transport/CorbaResponseWaitingRoomImpl.java
corba/src/share/classes/com/sun/corba/se/impl/transport/SocketOrChannelConnectionImpl.java
--- a/corba/src/share/classes/com/sun/corba/se/impl/transport/CorbaResponseWaitingRoomImpl.java	Wed Jul 05 18:19:33 2017 +0200
+++ b/corba/src/share/classes/com/sun/corba/se/impl/transport/CorbaResponseWaitingRoomImpl.java	Thu Aug 16 10:33:01 2012 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2004, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2012, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -25,7 +25,10 @@
 
 package com.sun.corba.se.impl.transport;
 
-import java.util.Hashtable;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.Map;
 
 import org.omg.CORBA.CompletionStatus;
 import org.omg.CORBA.SystemException;
@@ -68,7 +71,7 @@
 
     private CorbaConnection connection;
     // Maps requestId to an OutCallDesc.
-    private Hashtable out_calls = null; // REVISIT - use int hastable/map
+    final private Map<Integer, OutCallDesc> out_calls;
 
     public CorbaResponseWaitingRoomImpl(ORB orb, CorbaConnection connection)
     {
@@ -76,7 +79,8 @@
         wrapper = ORBUtilSystemException.get( orb,
             CORBALogDomains.RPC_TRANSPORT ) ;
         this.connection = connection;
-        out_calls = new Hashtable();
+        out_calls =
+            Collections.synchronizedMap(new HashMap<Integer, OutCallDesc>());
     }
 
     ////////////////////////////////////////////////////
@@ -139,7 +143,7 @@
             return null;
         }
 
-        OutCallDesc call = (OutCallDesc)out_calls.get(requestId);
+        OutCallDesc call = out_calls.get(requestId);
         if (call == null) {
             throw wrapper.nullOutCall(CompletionStatus.COMPLETED_MAYBE);
         }
@@ -197,7 +201,7 @@
         LocateReplyOrReplyMessage header = (LocateReplyOrReplyMessage)
             inputObject.getMessageHeader();
         Integer requestId = new Integer(header.getRequestId());
-        OutCallDesc call = (OutCallDesc) out_calls.get(requestId);
+        OutCallDesc call = out_calls.get(requestId);
 
         if (orb.transportDebugFlag) {
             dprint(".responseReceived: id/"
@@ -248,7 +252,6 @@
 
     public int numberRegistered()
     {
-        // Note: Hashtable.size() is not synchronized
         return out_calls.size();
     }
 
@@ -264,29 +267,41 @@
             dprint(".signalExceptionToAllWaiters: " + systemException);
         }
 
-        OutCallDesc call;
-        java.util.Enumeration e = out_calls.elements();
-        while(e.hasMoreElements()) {
-            call = (OutCallDesc) e.nextElement();
+        synchronized (out_calls) {
+            if (orb.transportDebugFlag) {
+                dprint(".signalExceptionToAllWaiters: out_calls size :" +
+                       out_calls.size());
+            }
 
-            synchronized(call.done){
-                // anything waiting for BufferManagerRead's fragment queue
-                // needs to be cancelled
-                CorbaMessageMediator corbaMsgMediator =
-                             (CorbaMessageMediator)call.messageMediator;
-                CDRInputObject inputObject =
-                           (CDRInputObject)corbaMsgMediator.getInputObject();
-                // IMPORTANT: If inputObject is null, then no need to tell
-                //            BufferManagerRead to cancel request processing.
-                if (inputObject != null) {
-                    BufferManagerReadStream bufferManager =
-                        (BufferManagerReadStream)inputObject.getBufferManager();
-                    int requestId = corbaMsgMediator.getRequestId();
-                    bufferManager.cancelProcessing(requestId);
+            for (OutCallDesc call : out_calls.values()) {
+                if (orb.transportDebugFlag) {
+                    dprint(".signalExceptionToAllWaiters: signaling " +
+                            call);
                 }
-                call.inputObject = null;
-                call.exception = systemException;
-                call.done.notify();
+                synchronized(call.done) {
+                    try {
+                        // anything waiting for BufferManagerRead's fragment queue
+                        // needs to be cancelled
+                        CorbaMessageMediator corbaMsgMediator =
+                                     (CorbaMessageMediator)call.messageMediator;
+                        CDRInputObject inputObject =
+                                   (CDRInputObject)corbaMsgMediator.getInputObject();
+                        // IMPORTANT: If inputObject is null, then no need to tell
+                        //            BufferManagerRead to cancel request processing.
+                        if (inputObject != null) {
+                            BufferManagerReadStream bufferManager =
+                                (BufferManagerReadStream)inputObject.getBufferManager();
+                            int requestId = corbaMsgMediator.getRequestId();
+                            bufferManager.cancelProcessing(requestId);
+                        }
+                    } catch (Exception e) {
+                    } finally {
+                        // attempt to wake up waiting threads in all cases
+                        call.inputObject = null;
+                        call.exception = systemException;
+                        call.done.notifyAll();
+                    }
+                }
             }
         }
     }
@@ -294,7 +309,7 @@
     public MessageMediator getMessageMediator(int requestId)
     {
         Integer id = new Integer(requestId);
-        OutCallDesc call = (OutCallDesc) out_calls.get(id);
+        OutCallDesc call = out_calls.get(id);
         if (call == null) {
             // This can happen when getting early reply fragments for a
             // request which has completed (e.g., client marshaling error).
--- a/corba/src/share/classes/com/sun/corba/se/impl/transport/SocketOrChannelConnectionImpl.java	Wed Jul 05 18:19:33 2017 +0200
+++ b/corba/src/share/classes/com/sun/corba/se/impl/transport/SocketOrChannelConnectionImpl.java	Thu Aug 16 10:33:01 2012 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2012, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -1521,7 +1521,7 @@
             // connection and give them the SystemException;
 
             responseWaitingRoom.signalExceptionToAllWaiters(systemException);
-
+        } finally {
             if (contactInfo != null) {
                 ((OutboundConnectionCache)getConnectionCache()).remove(contactInfo);
             } else if (acceptor != null) {
@@ -1542,7 +1542,6 @@
 
             writeUnlock();
 
-        } finally {
             if (orb.transportDebugFlag) {
                 dprint(".purgeCalls<-: "
                        + minor_code + "/" + die + "/" + lockHeld