6763340: memory leak in com.sun.corba.se.* classes

6873605: Missing finishedDispatch() call in ORBImpl causes test failures after 5u20 b04
Summary: Reviewed by Ken Cavanaugh
Reviewed-by: coffeys

src/share/classes/com/sun/corba/se/impl/interceptors/ClientRequestInfoImpl.java

@@ -74,6 +74,7 @@ import com.sun.corba.se.spi.ior.iiop.IIOPProfileTemplate;
 import com.sun.corba.se.spi.ior.iiop.GIOPVersion;
 import com.sun.corba.se.spi.orb.ORB;
 import com.sun.corba.se.spi.protocol.CorbaMessageMediator;
+import com.sun.corba.se.spi.protocol.RetryType;
 import com.sun.corba.se.spi.transport.CorbaContactInfo;
 import com.sun.corba.se.spi.transport.CorbaContactInfoList;
 import com.sun.corba.se.spi.transport.CorbaContactInfoListIterator;
@@ -110,7 +111,7 @@ public final class ClientRequestInfoImpl
 
     // The current retry request status.  True if this request is being
     // retried and this info object is to be reused, or false otherwise.
-    private boolean retryRequest;
+    private RetryType retryRequest;
 
     // The number of times this info object has been (re)used.  This is
     // incremented every time a request is retried, and decremented every
@@ -163,7 +164,8 @@ public final class ClientRequestInfoImpl
 
         // Please keep these in the same order that they're declared above.
 
-        retryRequest = false;
+        // 6763340
+        retryRequest = RetryType.NONE;
 
         // Do not reset entryCount because we need to know when to pop this
         // from the stack.
@@ -824,14 +826,15 @@ public final class ClientRequestInfoImpl
     /**
      * Set or reset the retry request flag.
      */
-    void setRetryRequest( boolean retryRequest ) {
+    void setRetryRequest( RetryType retryRequest ) {
         this.retryRequest = retryRequest;
     }
 
     /**
      * Retrieve the current retry request status.
      */
-    boolean getRetryRequest() {
+    RetryType getRetryRequest() {
+        // 6763340
         return this.retryRequest;
     }
 

src/share/classes/com/sun/corba/se/impl/interceptors/PIHandlerImpl.java

@@ -70,6 +70,7 @@ import com.sun.corba.se.spi.orbutil.closure.ClosureFactory;
 import com.sun.corba.se.spi.protocol.CorbaMessageMediator;
 import com.sun.corba.se.spi.protocol.ForwardException;
 import com.sun.corba.se.spi.protocol.PIHandler;
+import com.sun.corba.se.spi.protocol.RetryType;
 import com.sun.corba.se.spi.logging.CORBALogDomains;
 
 import com.sun.corba.se.impl.logging.InterceptorsSystemException;
@@ -372,9 +373,24 @@ public class PIHandlerImpl implements PIHandler
         }
     }
 
-    public Exception invokeClientPIEndingPoint(
-        int replyStatus, Exception exception )
-    {
+    // Needed when an error forces a retry AFTER initiateClientPIRequest
+    // but BEFORE invokeClientPIStartingPoint.
+    public Exception makeCompletedClientRequest( int replyStatus,
+        Exception exception ) {
+
+        // 6763340
+        return handleClientPIEndingPoint( replyStatus, exception, false ) ;
+    }
+
+    public Exception invokeClientPIEndingPoint( int replyStatus,
+        Exception exception ) {
+
+        // 6763340
+        return handleClientPIEndingPoint( replyStatus, exception, true ) ;
+    }
+
+    public Exception handleClientPIEndingPoint(
+        int replyStatus, Exception exception, boolean invokeEndingPoint ) {
         if( !hasClientInterceptors ) return exception;
         if( !isClientPIEnabledForThisThread() ) return exception;
 
@@ -388,24 +404,31 @@ public class PIHandlerImpl implements PIHandler
         ClientRequestInfoImpl info = peekClientRequestInfoImplStack();
         info.setReplyStatus( piReplyStatus );
         info.setException( exception );
-        interceptorInvoker.invokeClientInterceptorEndingPoint( info );
-        piReplyStatus = info.getReplyStatus();
+
+        if (invokeEndingPoint) {
+            // 6763340
+            interceptorInvoker.invokeClientInterceptorEndingPoint( info );
+            piReplyStatus = info.getReplyStatus();
+        }
 
         // Check reply status:
         if( (piReplyStatus == LOCATION_FORWARD.value) ||
-            (piReplyStatus == TRANSPORT_RETRY.value) )
-        {
+            (piReplyStatus == TRANSPORT_RETRY.value) ) {
             // If this is a forward or a retry, reset and reuse
             // info object:
             info.reset();
-            info.setRetryRequest( true );
+
+            // fix for 6763340:
+            if (invokeEndingPoint) {
+                info.setRetryRequest( RetryType.AFTER_RESPONSE ) ;
+            } else {
+                info.setRetryRequest( RetryType.BEFORE_RESPONSE ) ;
+            }
 
             // ... and return a RemarshalException so the orb internals know
             exception = new RemarshalException();
-        }
-        else if( (piReplyStatus == SYSTEM_EXCEPTION.value) ||
-                 (piReplyStatus == USER_EXCEPTION.value) )
-        {
+        } else if( (piReplyStatus == SYSTEM_EXCEPTION.value) ||
+                 (piReplyStatus == USER_EXCEPTION.value) ) {
             exception = info.getException();
         }
 
@@ -421,18 +444,21 @@ public class PIHandlerImpl implements PIHandler
         RequestInfoStack infoStack =
             (RequestInfoStack)threadLocalClientRequestInfoStack.get();
         ClientRequestInfoImpl info = null;
-        if( !infoStack.empty() ) info =
-            (ClientRequestInfoImpl)infoStack.peek();
 
-        if( !diiRequest && (info != null) && info.isDIIInitiate() ) {
+        if (!infoStack.empty() ) {
+            info = (ClientRequestInfoImpl)infoStack.peek();
+        }
+
+        if (!diiRequest && (info != null) && info.isDIIInitiate() ) {
             // In RequestImpl.doInvocation we already called
             // initiateClientPIRequest( true ), so ignore this initiate.
             info.setDIIInitiate( false );
-        }
-        else {
+        } else {
             // If there is no info object or if we are not retrying a request,
             // push a new ClientRequestInfoImpl on the stack:
-            if( (info == null) || !info.getRetryRequest() ) {
+
+            // 6763340: don't push unless this is not a retry
+            if( (info == null) || !info.getRetryRequest().isRetry() ) {
                 info = new ClientRequestInfoImpl( orb );
                 infoStack.push( info );
                 printPush();
@@ -442,9 +468,15 @@ public class PIHandlerImpl implements PIHandler
             // Reset the retry request flag so that recursive calls will
             // push a new info object, and bump up entry count so we know
             // when to pop this info object:
-            info.setRetryRequest( false );
+            info.setRetryRequest( RetryType.NONE );
             info.incrementEntryCount();
 
+            // KMC 6763340: I don't know why this wasn't set earlier,
+            // but we do not want a retry to pick up the previous
+            // reply status, so clear it here.  Most likely a new
+            // info was pushed before, so that this was not a problem.
+            info.setReplyStatus( RequestInfoImpl.UNINITIALIZED ) ;
+
             // If this is a DII request, make sure we ignore the next initiate.
             if( diiRequest ) {
                 info.setDIIInitiate( true );
@@ -457,25 +489,34 @@ public class PIHandlerImpl implements PIHandler
         if( !isClientPIEnabledForThisThread() ) return;
 
         ClientRequestInfoImpl info = peekClientRequestInfoImplStack();
-
-        // If the replyStatus has not yet been set, this is an indication
-        // that the ORB threw an exception before we had a chance to
-        // invoke the client interceptor ending points.
-        //
-        // _REVISIT_ We cannot handle any exceptions or ForwardRequests
-        // flagged by the ending points here because there is no way
-        // to gracefully handle this in any of the calling code.
-        // This is a rare corner case, so we will ignore this for now.
-        short replyStatus = info.getReplyStatus();
-        if( replyStatus == info.UNINITIALIZED ) {
-            invokeClientPIEndingPoint( ReplyMessage.SYSTEM_EXCEPTION,
-                wrapper.unknownRequestInvoke(
-                    CompletionStatus.COMPLETED_MAYBE ) ) ;
+        RetryType rt = info.getRetryRequest() ;
+
+        // fix for 6763340
+        if (!rt.equals( RetryType.BEFORE_RESPONSE )) {
+
+            // If the replyStatus has not yet been set, this is an indication
+            // that the ORB threw an exception before we had a chance to
+            // invoke the client interceptor ending points.
+            //
+            // _REVISIT_ We cannot handle any exceptions or ForwardRequests
+            // flagged by the ending points here because there is no way
+            // to gracefully handle this in any of the calling code.
+            // This is a rare corner case, so we will ignore this for now.
+            short replyStatus = info.getReplyStatus();
+            if (replyStatus == info.UNINITIALIZED ) {
+                invokeClientPIEndingPoint( ReplyMessage.SYSTEM_EXCEPTION,
+                    wrapper.unknownRequestInvoke(
+                        CompletionStatus.COMPLETED_MAYBE ) ) ;
+            }
         }
 
         // Decrement entry count, and if it is zero, pop it from the stack.
         info.decrementEntryCount();
-        if( info.getEntryCount() == 0 ) {
+
+        // fix for 6763340, and probably other cases (non-recursive retry)
+        if (info.getEntryCount() == 0 && !info.getRetryRequest().isRetry()) {
+            // RequestInfoStack<ClientRequestInfoImpl> infoStack =
+            //     threadLocalClientRequestInfoStack.get();
             RequestInfoStack infoStack =
                 (RequestInfoStack)threadLocalClientRequestInfoStack.get();
             infoStack.pop();

src/share/classes/com/sun/corba/se/impl/interceptors/PINoOpHandlerImpl.java

@@ -107,6 +107,11 @@ public class PINoOpHandlerImpl implements PIHandler
         return null;
     }
 
+    public Exception makeCompletedClientRequest(
+        int replyStatus, Exception exception ) {
+        return null;
+    }
+
     public void initiateClientPIRequest( boolean diiRequest ) {
     }
 

src/share/classes/com/sun/corba/se/impl/interceptors/RequestInfoImpl.java

@@ -187,7 +187,8 @@ public abstract class RequestInfoImpl
         startingPointCall = 0;
         intermediatePointCall = 0;
         endingPointCall = 0;
-        replyStatus = UNINITIALIZED;
+        // 6763340
+        setReplyStatus( UNINITIALIZED ) ;
         currentExecutionPoint = EXECUTION_POINT_STARTING;
         alreadyExecuted = false;
         connection = null;

src/share/classes/com/sun/corba/se/impl/orb/ORBImpl.java

@@ -1672,6 +1672,7 @@ public class ORBImpl extends com.sun.corba.se.spi.orb.ORB
     {
         StackImpl invocationInfoStack =
             (StackImpl)clientInvocationInfoStack.get();
+        int entryCount = -1;
         ClientInvocationInfo clientInvocationInfo = null;
         if (!invocationInfoStack.empty()) {
             clientInvocationInfo =
@@ -1680,8 +1681,12 @@ public class ORBImpl extends com.sun.corba.se.spi.orb.ORB
             throw wrapper.invocationInfoStackEmpty() ;
         }
         clientInvocationInfo.decrementEntryCount();
+        entryCount = clientInvocationInfo.getEntryCount();
         if (clientInvocationInfo.getEntryCount() == 0) {
-            invocationInfoStack.pop();
+            // 6763340: don't pop if this is a retry!
+            if (!clientInvocationInfo.isRetryInvocation()) {
+                invocationInfoStack.pop();
+            }
             finishedDispatch();
         }
     }

src/share/classes/com/sun/corba/se/impl/protocol/CorbaClientRequestDispatcherImpl.java

@@ -185,6 +185,7 @@ public class CorbaClientRequestDispatcherImpl
                             if(getContactInfoListIterator(orb).hasNext()) {
                                 contactInfo = (ContactInfo)
                                    getContactInfoListIterator(orb).next();
+                                unregisterWaiter(orb);
                                 return beginRequest(self, opName,
                                                     isOneWay, contactInfo);
                             } else {
@@ -292,10 +293,22 @@ public class CorbaClientRequestDispatcherImpl
             // ContactInfoList outside of subcontract.
             // Want to move that update to here.
             if (getContactInfoListIterator(orb).hasNext()) {
-                contactInfo = (ContactInfo)
-                    getContactInfoListIterator(orb).next();
+                contactInfo = (ContactInfo)getContactInfoListIterator(orb).next();
+                if (orb.subcontractDebugFlag) {
+                    dprint( "RemarshalException: hasNext true\ncontact info " + contactInfo );
+                }
+
+                // Fix for 6763340: Complete the first attempt before starting another.
+                orb.getPIHandler().makeCompletedClientRequest(
+                    ReplyMessage.LOCATION_FORWARD, null ) ;
+                unregisterWaiter(orb);
+                orb.getPIHandler().cleanupClientPIRequest() ;
+
                 return beginRequest(self, opName, isOneWay, contactInfo);
             } else {
+                if (orb.subcontractDebugFlag) {
+                    dprint( "RemarshalException: hasNext false" );
+                }
                 ORBUtilSystemException wrapper =
                     ORBUtilSystemException.get(orb,
                                                CORBALogDomains.RPC_PROTOCOL);

src/share/classes/com/sun/corba/se/spi/protocol/PIHandler.java

@@ -141,6 +141,27 @@ public interface PIHandler {
     Exception invokeClientPIEndingPoint(
         int replyStatus, Exception exception ) ;
 
+    /**
+     * Called when a retry is needed after initiateClientPIRequest but
+     * before invokeClientPIRequest.  In this case, we need to properly
+     * balance initiateClientPIRequest/cleanupClientPIRequest calls,
+     * but WITHOUT extraneous calls to invokeClientPIEndingPoint
+     * (see bug 6763340).
+     *
+     * @param replyStatus One of the constants in iiop.messages.ReplyMessage
+     *     indicating which reply status to set.
+     * @param exception The exception before ending interception points have
+     *     been invoked, or null if no exception at the moment.
+     * @return The exception to be thrown, after having gone through
+     *     all ending points, or null if there is no exception to be
+     *     thrown.  Note that this exception can be either the same or
+     *     different from the exception set using setClientPIException.
+     *     There are four possible return types: null (no exception),
+     *     SystemException, UserException, or RemarshalException.
+     */
+    Exception makeCompletedClientRequest(
+        int replyStatus, Exception exception ) ;
+
     /**
      * Invoked when a request is about to be created.  Must be called before
      * any of the setClientPI* methods so that a new info object can be

src/share/classes/com/sun/corba/se/spi/protocol/RetryType.java

+/*
+ * Copyright (c) 2010, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package com.sun.corba.se.spi.protocol ;
+
+// Introduce more information about WHY we are re-trying a request
+// so we can properly handle the two cases:
+// - BEFORE_RESPONSE means that the retry is caused by
+//   something that happened BEFORE the message was sent: either
+//   an exception from the SocketFactory, or one from the
+//   Client side send_request interceptor point.
+// - AFTER_RESPONSE means that the retry is a result either of the
+//   request sent to the server (from the response), or from the
+//   Client side receive_xxx interceptor point.
+public enum RetryType {
+    NONE( false ),
+    BEFORE_RESPONSE( true ),
+    AFTER_RESPONSE( true ) ;
+
+    private final boolean isRetry ;
+
+    RetryType( boolean isRetry ) {
+        this.isRetry = isRetry ;
+    }
+
+    public boolean isRetry() {
+        return this.isRetry ;
+    }
+} ;
+