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

Mon, 15 Nov 2010 10:47:48 -0800

author
robm
date
Mon, 15 Nov 2010 10:47:48 -0800
changeset 215
cff5a173ec1e
parent 214
f642c9ec81a0
child 216
4ab3c663d147
child 226
e0f7ed041196

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 file | annotate | diff | comparison | revisions
src/share/classes/com/sun/corba/se/impl/interceptors/PIHandlerImpl.java file | annotate | diff | comparison | revisions
src/share/classes/com/sun/corba/se/impl/interceptors/PINoOpHandlerImpl.java file | annotate | diff | comparison | revisions
src/share/classes/com/sun/corba/se/impl/interceptors/RequestInfoImpl.java file | annotate | diff | comparison | revisions
src/share/classes/com/sun/corba/se/impl/orb/ORBImpl.java file | annotate | diff | comparison | revisions
src/share/classes/com/sun/corba/se/impl/protocol/CorbaClientRequestDispatcherImpl.java file | annotate | diff | comparison | revisions
src/share/classes/com/sun/corba/se/spi/protocol/PIHandler.java file | annotate | diff | comparison | revisions
src/share/classes/com/sun/corba/se/spi/protocol/RetryType.java file | annotate | diff | comparison | revisions
     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 +

mercurial