Mon, 15 Nov 2010 10:47:48 -0800
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
1.1 --- a/src/share/classes/com/sun/corba/se/impl/interceptors/ClientRequestInfoImpl.java Mon Nov 15 10:46:40 2010 -0800 1.2 +++ b/src/share/classes/com/sun/corba/se/impl/interceptors/ClientRequestInfoImpl.java Mon Nov 15 10:47:48 2010 -0800 1.3 @@ -74,6 +74,7 @@ 1.4 import com.sun.corba.se.spi.ior.iiop.GIOPVersion; 1.5 import com.sun.corba.se.spi.orb.ORB; 1.6 import com.sun.corba.se.spi.protocol.CorbaMessageMediator; 1.7 +import com.sun.corba.se.spi.protocol.RetryType; 1.8 import com.sun.corba.se.spi.transport.CorbaContactInfo; 1.9 import com.sun.corba.se.spi.transport.CorbaContactInfoList; 1.10 import com.sun.corba.se.spi.transport.CorbaContactInfoListIterator; 1.11 @@ -110,7 +111,7 @@ 1.12 1.13 // The current retry request status. True if this request is being 1.14 // retried and this info object is to be reused, or false otherwise. 1.15 - private boolean retryRequest; 1.16 + private RetryType retryRequest; 1.17 1.18 // The number of times this info object has been (re)used. This is 1.19 // incremented every time a request is retried, and decremented every 1.20 @@ -163,7 +164,8 @@ 1.21 1.22 // Please keep these in the same order that they're declared above. 1.23 1.24 - retryRequest = false; 1.25 + // 6763340 1.26 + retryRequest = RetryType.NONE; 1.27 1.28 // Do not reset entryCount because we need to know when to pop this 1.29 // from the stack. 1.30 @@ -824,14 +826,15 @@ 1.31 /** 1.32 * Set or reset the retry request flag. 1.33 */ 1.34 - void setRetryRequest( boolean retryRequest ) { 1.35 + void setRetryRequest( RetryType retryRequest ) { 1.36 this.retryRequest = retryRequest; 1.37 } 1.38 1.39 /** 1.40 * Retrieve the current retry request status. 1.41 */ 1.42 - boolean getRetryRequest() { 1.43 + RetryType getRetryRequest() { 1.44 + // 6763340 1.45 return this.retryRequest; 1.46 } 1.47
2.1 --- a/src/share/classes/com/sun/corba/se/impl/interceptors/PIHandlerImpl.java Mon Nov 15 10:46:40 2010 -0800 2.2 +++ b/src/share/classes/com/sun/corba/se/impl/interceptors/PIHandlerImpl.java Mon Nov 15 10:47:48 2010 -0800 2.3 @@ -70,6 +70,7 @@ 2.4 import com.sun.corba.se.spi.protocol.CorbaMessageMediator; 2.5 import com.sun.corba.se.spi.protocol.ForwardException; 2.6 import com.sun.corba.se.spi.protocol.PIHandler; 2.7 +import com.sun.corba.se.spi.protocol.RetryType; 2.8 import com.sun.corba.se.spi.logging.CORBALogDomains; 2.9 2.10 import com.sun.corba.se.impl.logging.InterceptorsSystemException; 2.11 @@ -372,9 +373,24 @@ 2.12 } 2.13 } 2.14 2.15 - public Exception invokeClientPIEndingPoint( 2.16 - int replyStatus, Exception exception ) 2.17 - { 2.18 + // Needed when an error forces a retry AFTER initiateClientPIRequest 2.19 + // but BEFORE invokeClientPIStartingPoint. 2.20 + public Exception makeCompletedClientRequest( int replyStatus, 2.21 + Exception exception ) { 2.22 + 2.23 + // 6763340 2.24 + return handleClientPIEndingPoint( replyStatus, exception, false ) ; 2.25 + } 2.26 + 2.27 + public Exception invokeClientPIEndingPoint( int replyStatus, 2.28 + Exception exception ) { 2.29 + 2.30 + // 6763340 2.31 + return handleClientPIEndingPoint( replyStatus, exception, true ) ; 2.32 + } 2.33 + 2.34 + public Exception handleClientPIEndingPoint( 2.35 + int replyStatus, Exception exception, boolean invokeEndingPoint ) { 2.36 if( !hasClientInterceptors ) return exception; 2.37 if( !isClientPIEnabledForThisThread() ) return exception; 2.38 2.39 @@ -388,24 +404,31 @@ 2.40 ClientRequestInfoImpl info = peekClientRequestInfoImplStack(); 2.41 info.setReplyStatus( piReplyStatus ); 2.42 info.setException( exception ); 2.43 - interceptorInvoker.invokeClientInterceptorEndingPoint( info ); 2.44 - piReplyStatus = info.getReplyStatus(); 2.45 + 2.46 + if (invokeEndingPoint) { 2.47 + // 6763340 2.48 + interceptorInvoker.invokeClientInterceptorEndingPoint( info ); 2.49 + piReplyStatus = info.getReplyStatus(); 2.50 + } 2.51 2.52 // Check reply status: 2.53 if( (piReplyStatus == LOCATION_FORWARD.value) || 2.54 - (piReplyStatus == TRANSPORT_RETRY.value) ) 2.55 - { 2.56 + (piReplyStatus == TRANSPORT_RETRY.value) ) { 2.57 // If this is a forward or a retry, reset and reuse 2.58 // info object: 2.59 info.reset(); 2.60 - info.setRetryRequest( true ); 2.61 + 2.62 + // fix for 6763340: 2.63 + if (invokeEndingPoint) { 2.64 + info.setRetryRequest( RetryType.AFTER_RESPONSE ) ; 2.65 + } else { 2.66 + info.setRetryRequest( RetryType.BEFORE_RESPONSE ) ; 2.67 + } 2.68 2.69 // ... and return a RemarshalException so the orb internals know 2.70 exception = new RemarshalException(); 2.71 - } 2.72 - else if( (piReplyStatus == SYSTEM_EXCEPTION.value) || 2.73 - (piReplyStatus == USER_EXCEPTION.value) ) 2.74 - { 2.75 + } else if( (piReplyStatus == SYSTEM_EXCEPTION.value) || 2.76 + (piReplyStatus == USER_EXCEPTION.value) ) { 2.77 exception = info.getException(); 2.78 } 2.79 2.80 @@ -421,18 +444,21 @@ 2.81 RequestInfoStack infoStack = 2.82 (RequestInfoStack)threadLocalClientRequestInfoStack.get(); 2.83 ClientRequestInfoImpl info = null; 2.84 - if( !infoStack.empty() ) info = 2.85 - (ClientRequestInfoImpl)infoStack.peek(); 2.86 2.87 - if( !diiRequest && (info != null) && info.isDIIInitiate() ) { 2.88 + if (!infoStack.empty() ) { 2.89 + info = (ClientRequestInfoImpl)infoStack.peek(); 2.90 + } 2.91 + 2.92 + if (!diiRequest && (info != null) && info.isDIIInitiate() ) { 2.93 // In RequestImpl.doInvocation we already called 2.94 // initiateClientPIRequest( true ), so ignore this initiate. 2.95 info.setDIIInitiate( false ); 2.96 - } 2.97 - else { 2.98 + } else { 2.99 // If there is no info object or if we are not retrying a request, 2.100 // push a new ClientRequestInfoImpl on the stack: 2.101 - if( (info == null) || !info.getRetryRequest() ) { 2.102 + 2.103 + // 6763340: don't push unless this is not a retry 2.104 + if( (info == null) || !info.getRetryRequest().isRetry() ) { 2.105 info = new ClientRequestInfoImpl( orb ); 2.106 infoStack.push( info ); 2.107 printPush(); 2.108 @@ -442,9 +468,15 @@ 2.109 // Reset the retry request flag so that recursive calls will 2.110 // push a new info object, and bump up entry count so we know 2.111 // when to pop this info object: 2.112 - info.setRetryRequest( false ); 2.113 + info.setRetryRequest( RetryType.NONE ); 2.114 info.incrementEntryCount(); 2.115 2.116 + // KMC 6763340: I don't know why this wasn't set earlier, 2.117 + // but we do not want a retry to pick up the previous 2.118 + // reply status, so clear it here. Most likely a new 2.119 + // info was pushed before, so that this was not a problem. 2.120 + info.setReplyStatus( RequestInfoImpl.UNINITIALIZED ) ; 2.121 + 2.122 // If this is a DII request, make sure we ignore the next initiate. 2.123 if( diiRequest ) { 2.124 info.setDIIInitiate( true ); 2.125 @@ -457,25 +489,34 @@ 2.126 if( !isClientPIEnabledForThisThread() ) return; 2.127 2.128 ClientRequestInfoImpl info = peekClientRequestInfoImplStack(); 2.129 + RetryType rt = info.getRetryRequest() ; 2.130 2.131 - // If the replyStatus has not yet been set, this is an indication 2.132 - // that the ORB threw an exception before we had a chance to 2.133 - // invoke the client interceptor ending points. 2.134 - // 2.135 - // _REVISIT_ We cannot handle any exceptions or ForwardRequests 2.136 - // flagged by the ending points here because there is no way 2.137 - // to gracefully handle this in any of the calling code. 2.138 - // This is a rare corner case, so we will ignore this for now. 2.139 - short replyStatus = info.getReplyStatus(); 2.140 - if( replyStatus == info.UNINITIALIZED ) { 2.141 - invokeClientPIEndingPoint( ReplyMessage.SYSTEM_EXCEPTION, 2.142 - wrapper.unknownRequestInvoke( 2.143 - CompletionStatus.COMPLETED_MAYBE ) ) ; 2.144 + // fix for 6763340 2.145 + if (!rt.equals( RetryType.BEFORE_RESPONSE )) { 2.146 + 2.147 + // If the replyStatus has not yet been set, this is an indication 2.148 + // that the ORB threw an exception before we had a chance to 2.149 + // invoke the client interceptor ending points. 2.150 + // 2.151 + // _REVISIT_ We cannot handle any exceptions or ForwardRequests 2.152 + // flagged by the ending points here because there is no way 2.153 + // to gracefully handle this in any of the calling code. 2.154 + // This is a rare corner case, so we will ignore this for now. 2.155 + short replyStatus = info.getReplyStatus(); 2.156 + if (replyStatus == info.UNINITIALIZED ) { 2.157 + invokeClientPIEndingPoint( ReplyMessage.SYSTEM_EXCEPTION, 2.158 + wrapper.unknownRequestInvoke( 2.159 + CompletionStatus.COMPLETED_MAYBE ) ) ; 2.160 + } 2.161 } 2.162 2.163 // Decrement entry count, and if it is zero, pop it from the stack. 2.164 info.decrementEntryCount(); 2.165 - if( info.getEntryCount() == 0 ) { 2.166 + 2.167 + // fix for 6763340, and probably other cases (non-recursive retry) 2.168 + if (info.getEntryCount() == 0 && !info.getRetryRequest().isRetry()) { 2.169 + // RequestInfoStack<ClientRequestInfoImpl> infoStack = 2.170 + // threadLocalClientRequestInfoStack.get(); 2.171 RequestInfoStack infoStack = 2.172 (RequestInfoStack)threadLocalClientRequestInfoStack.get(); 2.173 infoStack.pop();
3.1 --- a/src/share/classes/com/sun/corba/se/impl/interceptors/PINoOpHandlerImpl.java Mon Nov 15 10:46:40 2010 -0800 3.2 +++ b/src/share/classes/com/sun/corba/se/impl/interceptors/PINoOpHandlerImpl.java Mon Nov 15 10:47:48 2010 -0800 3.3 @@ -107,6 +107,11 @@ 3.4 return null; 3.5 } 3.6 3.7 + public Exception makeCompletedClientRequest( 3.8 + int replyStatus, Exception exception ) { 3.9 + return null; 3.10 + } 3.11 + 3.12 public void initiateClientPIRequest( boolean diiRequest ) { 3.13 } 3.14
4.1 --- a/src/share/classes/com/sun/corba/se/impl/interceptors/RequestInfoImpl.java Mon Nov 15 10:46:40 2010 -0800 4.2 +++ b/src/share/classes/com/sun/corba/se/impl/interceptors/RequestInfoImpl.java Mon Nov 15 10:47:48 2010 -0800 4.3 @@ -187,7 +187,8 @@ 4.4 startingPointCall = 0; 4.5 intermediatePointCall = 0; 4.6 endingPointCall = 0; 4.7 - replyStatus = UNINITIALIZED; 4.8 + // 6763340 4.9 + setReplyStatus( UNINITIALIZED ) ; 4.10 currentExecutionPoint = EXECUTION_POINT_STARTING; 4.11 alreadyExecuted = false; 4.12 connection = null;
5.1 --- a/src/share/classes/com/sun/corba/se/impl/orb/ORBImpl.java Mon Nov 15 10:46:40 2010 -0800 5.2 +++ b/src/share/classes/com/sun/corba/se/impl/orb/ORBImpl.java Mon Nov 15 10:47:48 2010 -0800 5.3 @@ -1672,6 +1672,7 @@ 5.4 { 5.5 StackImpl invocationInfoStack = 5.6 (StackImpl)clientInvocationInfoStack.get(); 5.7 + int entryCount = -1; 5.8 ClientInvocationInfo clientInvocationInfo = null; 5.9 if (!invocationInfoStack.empty()) { 5.10 clientInvocationInfo = 5.11 @@ -1680,8 +1681,12 @@ 5.12 throw wrapper.invocationInfoStackEmpty() ; 5.13 } 5.14 clientInvocationInfo.decrementEntryCount(); 5.15 + entryCount = clientInvocationInfo.getEntryCount(); 5.16 if (clientInvocationInfo.getEntryCount() == 0) { 5.17 - invocationInfoStack.pop(); 5.18 + // 6763340: don't pop if this is a retry! 5.19 + if (!clientInvocationInfo.isRetryInvocation()) { 5.20 + invocationInfoStack.pop(); 5.21 + } 5.22 finishedDispatch(); 5.23 } 5.24 }
6.1 --- a/src/share/classes/com/sun/corba/se/impl/protocol/CorbaClientRequestDispatcherImpl.java Mon Nov 15 10:46:40 2010 -0800 6.2 +++ b/src/share/classes/com/sun/corba/se/impl/protocol/CorbaClientRequestDispatcherImpl.java Mon Nov 15 10:47:48 2010 -0800 6.3 @@ -185,6 +185,7 @@ 6.4 if(getContactInfoListIterator(orb).hasNext()) { 6.5 contactInfo = (ContactInfo) 6.6 getContactInfoListIterator(orb).next(); 6.7 + unregisterWaiter(orb); 6.8 return beginRequest(self, opName, 6.9 isOneWay, contactInfo); 6.10 } else { 6.11 @@ -292,10 +293,22 @@ 6.12 // ContactInfoList outside of subcontract. 6.13 // Want to move that update to here. 6.14 if (getContactInfoListIterator(orb).hasNext()) { 6.15 - contactInfo = (ContactInfo) 6.16 - getContactInfoListIterator(orb).next(); 6.17 + contactInfo = (ContactInfo)getContactInfoListIterator(orb).next(); 6.18 + if (orb.subcontractDebugFlag) { 6.19 + dprint( "RemarshalException: hasNext true\ncontact info " + contactInfo ); 6.20 + } 6.21 + 6.22 + // Fix for 6763340: Complete the first attempt before starting another. 6.23 + orb.getPIHandler().makeCompletedClientRequest( 6.24 + ReplyMessage.LOCATION_FORWARD, null ) ; 6.25 + unregisterWaiter(orb); 6.26 + orb.getPIHandler().cleanupClientPIRequest() ; 6.27 + 6.28 return beginRequest(self, opName, isOneWay, contactInfo); 6.29 } else { 6.30 + if (orb.subcontractDebugFlag) { 6.31 + dprint( "RemarshalException: hasNext false" ); 6.32 + } 6.33 ORBUtilSystemException wrapper = 6.34 ORBUtilSystemException.get(orb, 6.35 CORBALogDomains.RPC_PROTOCOL);
7.1 --- a/src/share/classes/com/sun/corba/se/spi/protocol/PIHandler.java Mon Nov 15 10:46:40 2010 -0800 7.2 +++ b/src/share/classes/com/sun/corba/se/spi/protocol/PIHandler.java Mon Nov 15 10:47:48 2010 -0800 7.3 @@ -142,6 +142,27 @@ 7.4 int replyStatus, Exception exception ) ; 7.5 7.6 /** 7.7 + * Called when a retry is needed after initiateClientPIRequest but 7.8 + * before invokeClientPIRequest. In this case, we need to properly 7.9 + * balance initiateClientPIRequest/cleanupClientPIRequest calls, 7.10 + * but WITHOUT extraneous calls to invokeClientPIEndingPoint 7.11 + * (see bug 6763340). 7.12 + * 7.13 + * @param replyStatus One of the constants in iiop.messages.ReplyMessage 7.14 + * indicating which reply status to set. 7.15 + * @param exception The exception before ending interception points have 7.16 + * been invoked, or null if no exception at the moment. 7.17 + * @return The exception to be thrown, after having gone through 7.18 + * all ending points, or null if there is no exception to be 7.19 + * thrown. Note that this exception can be either the same or 7.20 + * different from the exception set using setClientPIException. 7.21 + * There are four possible return types: null (no exception), 7.22 + * SystemException, UserException, or RemarshalException. 7.23 + */ 7.24 + Exception makeCompletedClientRequest( 7.25 + int replyStatus, Exception exception ) ; 7.26 + 7.27 + /** 7.28 * Invoked when a request is about to be created. Must be called before 7.29 * any of the setClientPI* methods so that a new info object can be 7.30 * prepared for information collection.
8.1 --- /dev/null Thu Jan 01 00:00:00 1970 +0000 8.2 +++ b/src/share/classes/com/sun/corba/se/spi/protocol/RetryType.java Mon Nov 15 10:47:48 2010 -0800 8.3 @@ -0,0 +1,52 @@ 8.4 +/* 8.5 + * Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved. 8.6 + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. 8.7 + * 8.8 + * This code is free software; you can redistribute it and/or modify it 8.9 + * under the terms of the GNU General Public License version 2 only, as 8.10 + * published by the Free Software Foundation. Oracle designates this 8.11 + * particular file as subject to the "Classpath" exception as provided 8.12 + * by Oracle in the LICENSE file that accompanied this code. 8.13 + * 8.14 + * This code is distributed in the hope that it will be useful, but WITHOUT 8.15 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or 8.16 + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License 8.17 + * version 2 for more details (a copy is included in the LICENSE file that 8.18 + * accompanied this code). 8.19 + * 8.20 + * You should have received a copy of the GNU General Public License version 8.21 + * 2 along with this work; if not, write to the Free Software Foundation, 8.22 + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. 8.23 + * 8.24 + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA 8.25 + * or visit www.oracle.com if you need additional information or have any 8.26 + * questions. 8.27 + */ 8.28 + 8.29 +package com.sun.corba.se.spi.protocol ; 8.30 + 8.31 +// Introduce more information about WHY we are re-trying a request 8.32 +// so we can properly handle the two cases: 8.33 +// - BEFORE_RESPONSE means that the retry is caused by 8.34 +// something that happened BEFORE the message was sent: either 8.35 +// an exception from the SocketFactory, or one from the 8.36 +// Client side send_request interceptor point. 8.37 +// - AFTER_RESPONSE means that the retry is a result either of the 8.38 +// request sent to the server (from the response), or from the 8.39 +// Client side receive_xxx interceptor point. 8.40 +public enum RetryType { 8.41 + NONE( false ), 8.42 + BEFORE_RESPONSE( true ), 8.43 + AFTER_RESPONSE( true ) ; 8.44 + 8.45 + private final boolean isRetry ; 8.46 + 8.47 + RetryType( boolean isRetry ) { 8.48 + this.isRetry = isRetry ; 8.49 + } 8.50 + 8.51 + public boolean isRetry() { 8.52 + return this.isRetry ; 8.53 + } 8.54 +} ; 8.55 +