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
--- a/corba/src/share/classes/com/sun/corba/se/impl/interceptors/ClientRequestInfoImpl.java Mon Nov 15 10:46:40 2010 -0800
+++ b/corba/src/share/classes/com/sun/corba/se/impl/interceptors/ClientRequestInfoImpl.java Mon Nov 15 10:47:48 2010 -0800
@@ -74,6 +74,7 @@
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 @@
// 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 @@
// 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 @@
/**
* 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;
}
--- a/corba/src/share/classes/com/sun/corba/se/impl/interceptors/PIHandlerImpl.java Mon Nov 15 10:46:40 2010 -0800
+++ b/corba/src/share/classes/com/sun/corba/se/impl/interceptors/PIHandlerImpl.java Mon Nov 15 10:47:48 2010 -0800
@@ -70,6 +70,7 @@
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 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 @@
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 @@
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 @@
// 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 @@
if( !isClientPIEnabledForThisThread() ) return;
ClientRequestInfoImpl info = peekClientRequestInfoImplStack();
+ RetryType rt = info.getRetryRequest() ;
- // 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 ) ) ;
+ // 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();
--- a/corba/src/share/classes/com/sun/corba/se/impl/interceptors/PINoOpHandlerImpl.java Mon Nov 15 10:46:40 2010 -0800
+++ b/corba/src/share/classes/com/sun/corba/se/impl/interceptors/PINoOpHandlerImpl.java Mon Nov 15 10:47:48 2010 -0800
@@ -107,6 +107,11 @@
return null;
}
+ public Exception makeCompletedClientRequest(
+ int replyStatus, Exception exception ) {
+ return null;
+ }
+
public void initiateClientPIRequest( boolean diiRequest ) {
}
--- a/corba/src/share/classes/com/sun/corba/se/impl/interceptors/RequestInfoImpl.java Mon Nov 15 10:46:40 2010 -0800
+++ b/corba/src/share/classes/com/sun/corba/se/impl/interceptors/RequestInfoImpl.java Mon Nov 15 10:47:48 2010 -0800
@@ -187,7 +187,8 @@
startingPointCall = 0;
intermediatePointCall = 0;
endingPointCall = 0;
- replyStatus = UNINITIALIZED;
+ // 6763340
+ setReplyStatus( UNINITIALIZED ) ;
currentExecutionPoint = EXECUTION_POINT_STARTING;
alreadyExecuted = false;
connection = null;
--- a/corba/src/share/classes/com/sun/corba/se/impl/orb/ORBImpl.java Mon Nov 15 10:46:40 2010 -0800
+++ b/corba/src/share/classes/com/sun/corba/se/impl/orb/ORBImpl.java Mon Nov 15 10:47:48 2010 -0800
@@ -1672,6 +1672,7 @@
{
StackImpl invocationInfoStack =
(StackImpl)clientInvocationInfoStack.get();
+ int entryCount = -1;
ClientInvocationInfo clientInvocationInfo = null;
if (!invocationInfoStack.empty()) {
clientInvocationInfo =
@@ -1680,8 +1681,12 @@
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();
}
}
--- a/corba/src/share/classes/com/sun/corba/se/impl/protocol/CorbaClientRequestDispatcherImpl.java Mon Nov 15 10:46:40 2010 -0800
+++ b/corba/src/share/classes/com/sun/corba/se/impl/protocol/CorbaClientRequestDispatcherImpl.java Mon Nov 15 10:47:48 2010 -0800
@@ -185,6 +185,7 @@
if(getContactInfoListIterator(orb).hasNext()) {
contactInfo = (ContactInfo)
getContactInfoListIterator(orb).next();
+ unregisterWaiter(orb);
return beginRequest(self, opName,
isOneWay, contactInfo);
} else {
@@ -292,10 +293,22 @@
// 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);
--- a/corba/src/share/classes/com/sun/corba/se/spi/protocol/PIHandler.java Mon Nov 15 10:46:40 2010 -0800
+++ b/corba/src/share/classes/com/sun/corba/se/spi/protocol/PIHandler.java Mon Nov 15 10:47:48 2010 -0800
@@ -142,6 +142,27 @@
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
* prepared for information collection.
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/corba/src/share/classes/com/sun/corba/se/spi/protocol/RetryType.java Mon Nov 15 10:47:48 2010 -0800
@@ -0,0 +1,52 @@
+/*
+ * 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 ;
+ }
+} ;
+