Mercurial > hg > openjdk > jdk6 > corba
changeset 97:c5c543cb91fc
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
line wrap: on
line diff
--- a/src/share/classes/com/sun/corba/se/impl/interceptors/ClientRequestInfoImpl.java Sat Nov 02 01:07:53 2013 +0000 +++ b/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/src/share/classes/com/sun/corba/se/impl/interceptors/PIHandlerImpl.java Sat Nov 02 01:07:53 2013 +0000 +++ b/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; @@ -371,9 +372,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; @@ -387,24 +403,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(); } @@ -420,18 +443,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(); @@ -441,9 +467,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 ); @@ -456,25 +488,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/src/share/classes/com/sun/corba/se/impl/interceptors/PINoOpHandlerImpl.java Sat Nov 02 01:07:53 2013 +0000 +++ b/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/src/share/classes/com/sun/corba/se/impl/interceptors/RequestInfoImpl.java Sat Nov 02 01:07:53 2013 +0000 +++ b/src/share/classes/com/sun/corba/se/impl/interceptors/RequestInfoImpl.java Mon Nov 15 10:47:48 2010 -0800 @@ -188,7 +188,8 @@ startingPointCall = 0; intermediatePointCall = 0; endingPointCall = 0; - replyStatus = UNINITIALIZED; + // 6763340 + setReplyStatus( UNINITIALIZED ) ; currentExecutionPoint = EXECUTION_POINT_STARTING; alreadyExecuted = false; connection = null;
--- a/src/share/classes/com/sun/corba/se/impl/orb/ORBImpl.java Sat Nov 02 01:07:53 2013 +0000 +++ b/src/share/classes/com/sun/corba/se/impl/orb/ORBImpl.java Mon Nov 15 10:47:48 2010 -0800 @@ -1671,6 +1671,7 @@ { StackImpl invocationInfoStack = (StackImpl)clientInvocationInfoStack.get(); + int entryCount = -1; ClientInvocationInfo clientInvocationInfo = null; if (!invocationInfoStack.empty()) { clientInvocationInfo = @@ -1679,8 +1680,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/src/share/classes/com/sun/corba/se/impl/protocol/CorbaClientRequestDispatcherImpl.java Sat Nov 02 01:07:53 2013 +0000 +++ b/src/share/classes/com/sun/corba/se/impl/protocol/CorbaClientRequestDispatcherImpl.java Mon Nov 15 10:47:48 2010 -0800 @@ -186,6 +186,7 @@ if(getContactInfoListIterator(orb).hasNext()) { contactInfo = (ContactInfo) getContactInfoListIterator(orb).next(); + unregisterWaiter(orb); return beginRequest(self, opName, isOneWay, contactInfo); } else { @@ -293,10 +294,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/src/share/classes/com/sun/corba/se/spi/protocol/PIHandler.java Sat Nov 02 01:07:53 2013 +0000 +++ b/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/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 ; + } +} ; +