directory-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From trus...@apache.org
Subject svn commit: r354680 - in /directory/network/trunk/src/java/org/apache/mina: common/CloseFuture.java common/IoFuture.java common/IoSession.java common/support/BaseIoSession.java filter/SSLFilter.java filter/support/SSLHandler.java
Date Wed, 07 Dec 2005 02:56:52 GMT
Author: trustin
Date: Tue Dec  6 18:56:45 2005
New Revision: 354680

URL: http://svn.apache.org/viewcvs?rev=354680&view=rev
Log:
Resolved issue: DIRMINA-130 (SSLFilter has to dispose SSLEngine when the filter is removed
from a chain.)
* Renamed SSLHandler.release() to destroy() and added more cleanup code
Resolved issue: DIRMINA-138 (Deadlock in SSLFilter)
* Added SSLHandler.init() so SSLHandler instance can be reused over and over
* BaseIoSession doesn't hold lock for a long time anymore.
* SSLFilter uses SSLHandler instance to acquire a lock now. (instead of IoSession)

Modified:
    directory/network/trunk/src/java/org/apache/mina/common/CloseFuture.java
    directory/network/trunk/src/java/org/apache/mina/common/IoFuture.java
    directory/network/trunk/src/java/org/apache/mina/common/IoSession.java
    directory/network/trunk/src/java/org/apache/mina/common/support/BaseIoSession.java
    directory/network/trunk/src/java/org/apache/mina/filter/SSLFilter.java
    directory/network/trunk/src/java/org/apache/mina/filter/support/SSLHandler.java

Modified: directory/network/trunk/src/java/org/apache/mina/common/CloseFuture.java
URL: http://svn.apache.org/viewcvs/directory/network/trunk/src/java/org/apache/mina/common/CloseFuture.java?rev=354680&r1=354679&r2=354680&view=diff
==============================================================================
--- directory/network/trunk/src/java/org/apache/mina/common/CloseFuture.java (original)
+++ directory/network/trunk/src/java/org/apache/mina/common/CloseFuture.java Tue Dec  6 18:56:45
2005
@@ -43,6 +43,11 @@
     {
     }
     
+    public CloseFuture( Object lock )
+    {
+        super( lock );
+    }
+    
     /**
      * Returns <tt>true</tt> if the close request is finished and the session
is closed.
      */

Modified: directory/network/trunk/src/java/org/apache/mina/common/IoFuture.java
URL: http://svn.apache.org/viewcvs/directory/network/trunk/src/java/org/apache/mina/common/IoFuture.java?rev=354680&r1=354679&r2=354680&view=diff
==============================================================================
--- directory/network/trunk/src/java/org/apache/mina/common/IoFuture.java (original)
+++ directory/network/trunk/src/java/org/apache/mina/common/IoFuture.java Tue Dec  6 18:56:45
2005
@@ -56,6 +56,11 @@
         this.lock = this;
     }
     
+    protected IoFuture( Object lock )
+    {
+        this.lock = lock;
+    }
+    
     /**
      * Wait for the asynchronous operation to end.
      */

Modified: directory/network/trunk/src/java/org/apache/mina/common/IoSession.java
URL: http://svn.apache.org/viewcvs/directory/network/trunk/src/java/org/apache/mina/common/IoSession.java?rev=354680&r1=354679&r2=354680&view=diff
==============================================================================
--- directory/network/trunk/src/java/org/apache/mina/common/IoSession.java (original)
+++ directory/network/trunk/src/java/org/apache/mina/common/IoSession.java Tue Dec  6 18:56:45
2005
@@ -37,18 +37,7 @@
  * 
  * <h3>Thread Safety</h3>
  * <p>
- * {@link IoSession} is thread safe.  {@link #close()} and {@link #write(Object)}
- * operations are performed exclusively acquiring its {@link IoSession} instance
- * as a lock.  You can also use {@link IoSession} instance to synchronize your code:
- * <pre>
- *   IoSession session = ...;
- *   synchronized( session )
- *   {
- *       // Some code that may not be executed while <tt>session.close()</tt>
- *       // or <tt>session.write</tt> is being executed.
- *       ...
- *   }
- * </pre>
+ * {@link IoSession} is thread safe.
  * </p>
  *   
  * @author The Apache Directory Project (dev@directory.apache.org)

Modified: directory/network/trunk/src/java/org/apache/mina/common/support/BaseIoSession.java
URL: http://svn.apache.org/viewcvs/directory/network/trunk/src/java/org/apache/mina/common/support/BaseIoSession.java?rev=354680&r1=354679&r2=354680&view=diff
==============================================================================
--- directory/network/trunk/src/java/org/apache/mina/common/support/BaseIoSession.java (original)
+++ directory/network/trunk/src/java/org/apache/mina/common/support/BaseIoSession.java Tue
Dec  6 18:56:45 2005
@@ -45,7 +45,7 @@
     /** 
      * A future that will be set 'closed' when the connection is closed.
      */
-    private final CloseFuture closeFuture = new CloseFuture();
+    private final CloseFuture closeFuture = new CloseFuture( this );
     private boolean closing;
 
     // Configuration variables
@@ -94,14 +94,17 @@
         return closeFuture;
     }
     
-    public synchronized CloseFuture close()
+    public CloseFuture close()
     {
-        if( !closing )
+        synchronized( this )
         {
-            closing = true;
-            close0( closeFuture );
+            if( !closing )
+            {
+                closing = true;
+            }
         }
 
+        close0( closeFuture );
         return closeFuture;
     }
 
@@ -117,18 +120,19 @@
         closeFuture.setClosed();
     }
     
-    public synchronized WriteFuture write( Object message )
+    public WriteFuture write( Object message )
     {
-        WriteFuture future;
-        if( isClosing() || !isConnected() )
+        synchronized( this )
         {
-            future = WriteFuture.newNotWrittenFuture();
-        }
-        else
-        {
-            future = new WriteFuture();
-            write0( new WriteRequest( message, future ) );
+            if( isClosing() || !isConnected() )
+            {
+                return WriteFuture.newNotWrittenFuture();
+            }
         }
+
+        WriteFuture future = new WriteFuture();
+        write0( new WriteRequest( message, future ) );
+        
         return future;
     }
     

Modified: directory/network/trunk/src/java/org/apache/mina/filter/SSLFilter.java
URL: http://svn.apache.org/viewcvs/directory/network/trunk/src/java/org/apache/mina/filter/SSLFilter.java?rev=354680&r1=354679&r2=354680&view=diff
==============================================================================
--- directory/network/trunk/src/java/org/apache/mina/filter/SSLFilter.java (original)
+++ directory/network/trunk/src/java/org/apache/mina/filter/SSLFilter.java Tue Dec  6 18:56:45
2005
@@ -158,28 +158,21 @@
      */
     public boolean startSSL( IoSession session ) throws SSLException
     {
-        synchronized( session )
+        SSLHandler handler = getSSLSessionHandler( session );
+        synchronized( handler )
         {
-            SSLHandler handler = getSSLSessionHandler( session );
-            if( handler != null )
+            if( handler.isOutboundDone() )
             {
-                if( handler.getParent() != this )
-                {
-                    throw new IllegalArgumentException( "Not managed by this filter." );
-                }
-    
-                if( handler.isOutboundDone() )
-                {
-                    session.removeAttribute( SSL_HANDLER );
-                }
-                else
-                {
-                    return false;
-                }
+                NextFilter nextFilter = ( NextFilter ) session.getAttribute( NEXT_FILTER
);
+                handler.destroy();
+                handler.init();
+                handler.handshake( nextFilter );
+                return true;
+            }
+            else
+            {
+                return false;
             }
-            
-            createSSLSessionHandler( ( NextFilter ) session.getAttribute( NEXT_FILTER ),
session );
-            return true;
         }
     }
     
@@ -191,14 +184,9 @@
      */
     public boolean isSSLStarted( IoSession session )
     {
-        synchronized( session )
+        SSLHandler handler = getSSLSessionHandler( session );
+        synchronized( handler )
         {
-            SSLHandler handler = getSSLSessionHandler( session );
-            if( handler == null )
-            {
-                return true;
-            }
-            
             return !handler.isOutboundDone();
         }
     }
@@ -213,21 +201,10 @@
      */
     public WriteFuture stopSSL( IoSession session ) throws SSLException
     {
-        synchronized( session )
+        SSLHandler handler = getSSLSessionHandler( session );
+        NextFilter nextFilter = ( NextFilter ) session.getAttribute( NEXT_FILTER );
+        synchronized( handler )
         {
-            SSLHandler handler = getSSLSessionHandler( session );
-            if( handler == null )
-            {
-                // Return a dummy future to prevent NFE.
-                return WriteFuture.newNotWrittenFuture();
-            }
-            
-            if( handler.getParent() != this )
-            {
-                throw new IllegalArgumentException( "Not managed by this filter." );
-            }
-            
-            NextFilter nextFilter = ( NextFilter ) session.getAttribute( NEXT_FILTER );
             return initiateClosure( nextFilter, session );
         }
     }
@@ -340,38 +317,32 @@
     
     public void onPostAdd( IoFilterChain parent, String name, NextFilter nextFilter ) throws
SSLException
     {
-        parent.getSession().setAttribute( NEXT_FILTER, nextFilter );
-        createSSLSessionHandler( nextFilter, parent.getSession() );
+        IoSession session = parent.getSession();
+        session.setAttribute( NEXT_FILTER, nextFilter );
+        
+        // Create an SSL handler and start handshake.
+        SSLHandler handler =
+            new SSLHandler( this, sslContext, session );
+        session.setAttribute( SSL_HANDLER, handler );
+        handler.handshake( nextFilter );
     }
     
     public void onPreRemove( IoFilterChain parent, String name, NextFilter nextFilter ) throws
SSLException
     {
-        stopSSL( parent.getSession() ).join();
+        IoSession session = parent.getSession();
+        WriteFuture future = stopSSL( session );
+        session.removeAttribute( NEXT_FILTER );
+        session.removeAttribute( SSL_HANDLER );
+        future.join();
     }
 
     // IoFilter impl.
-
-    public void sessionOpened( NextFilter nextFilter, IoSession session ) throws SSLException
-    {
-        synchronized( session )
-        {
-            if( !isSSLStarted( session ) )
-            {
-                nextFilter.sessionOpened( session );
-                return;
-            }
-    
-            // Create an SSL handler
-            createSSLSessionHandler( nextFilter, session );
-            nextFilter.sessionOpened( session );
-        }
-    }
-
     public void sessionClosed( NextFilter nextFilter, IoSession session ) throws SSLException
     {
+        SSLHandler handler = getSSLSessionHandler( session );
         try
         {
-            synchronized( session )
+            synchronized( handler )
             {
                 if( isSSLStarted( session ) )
                 {
@@ -379,10 +350,10 @@
                     {
                         SessionLog.debug( session, " Closed: " + getSSLSessionHandler( session
) );
                     }
-                
-                    // release resources
-                    releaseSSLSessionHandler( session );
                 }
+                
+                // release resources
+                handler.destroy();
             }
         }
         finally
@@ -395,13 +366,12 @@
     public void messageReceived( NextFilter nextFilter, IoSession session,
                                  Object message ) throws SSLException
     {
-        synchronized( session )
+        SSLHandler handler = getSSLSessionHandler( session );
+        synchronized( handler )
         {
-            SSLHandler sslHandler = createSSLSessionHandler( nextFilter, session );
-    
             if( !isSSLStarted( session ) )
             {
-                if( sslHandler != null && sslHandler.isInboundDone() )
+                if( handler.isInboundDone() )
                 {
                     nextFilter.messageReceived( session, message );
                     return;
@@ -409,61 +379,53 @@
             }
     
             ByteBuffer buf = ( ByteBuffer ) message;
-            sslHandler = createSSLSessionHandler( nextFilter, session );
-            if( sslHandler != null )
+            if( SessionLog.isDebugEnabled( session ) )
             {
-                if( SessionLog.isDebugEnabled( session ) )
-                {
-                    SessionLog.debug( session, " Data Read: " + sslHandler + " (" + buf+
')' );
-                }
+                SessionLog.debug( session, " Data Read: " + handler + " (" + buf+ ')' );
+            }
 
-                try
-                {
-                    // forward read encrypted data to SSL handler
-                    sslHandler.messageReceived( nextFilter, buf.buf() );
+            try
+            {
+                // forward read encrypted data to SSL handler
+                handler.messageReceived( nextFilter, buf.buf() );
 
-                    // Handle data to be forwarded to application or written to net
-                    handleSSLData( nextFilter, sslHandler );
+                // Handle data to be forwarded to application or written to net
+                handleSSLData( nextFilter, handler );
 
-                    if( sslHandler.isInboundDone() )
+                if( handler.isInboundDone() )
+                {
+                    if( handler.isOutboundDone() )
                     {
-                        if( sslHandler.isOutboundDone() )
-                        {
-                            if( SessionLog.isDebugEnabled( session ) )
-                            {
-                                SessionLog.debug(
-                                        session, " SSL Session closed." );
-                            }
-                            
-                            releaseSSLSessionHandler( session );
-                        }
-                        else
-                        {
-                            initiateClosure( nextFilter, session );
-                        }
-
-                        if( buf.hasRemaining() )
+                        if( SessionLog.isDebugEnabled( session ) )
                         {
-                            nextFilter.messageReceived( session, buf );
+                            SessionLog.debug(
+                                    session, " SSL Session closed." );
                         }
+                        
+                        handler.destroy();
                     }
-                }
-                catch( SSLException ssle )
-                {
-                    if( !sslHandler.isInitialHandshakeComplete() )
+                    else
                     {
-                        SSLException newSSLE = new SSLHandshakeException(
-                                "Initial SSL handshake failed." );
-                        newSSLE.initCause( ssle );
-                        ssle = newSSLE;
+                        initiateClosure( nextFilter, session );
                     }
 
-                    throw ssle;
+                    if( buf.hasRemaining() )
+                    {
+                        nextFilter.messageReceived( session, buf );
+                    }
                 }
             }
-            else
+            catch( SSLException ssle )
             {
-                nextFilter.messageReceived( session, buf );
+                if( !handler.isInitialHandshakeComplete() )
+                {
+                    SSLException newSSLE = new SSLHandshakeException(
+                            "Initial SSL handshake failed." );
+                    newSSLE.initCause( ssle );
+                    ssle = newSSLE;
+                }
+
+                throw ssle;
             }
         }
     }
@@ -485,7 +447,8 @@
 
     public void filterWrite( NextFilter nextFilter, IoSession session, WriteRequest writeRequest
) throws SSLException
     {
-        synchronized( session )
+        SSLHandler handler = getSSLSessionHandler( session );
+        synchronized( handler )
         {
             if( !isSSLStarted( session ) )
             {
@@ -505,8 +468,6 @@
             // Otherwise, encrypt the buffer.
             ByteBuffer buf = ( ByteBuffer ) writeRequest.getMessage();
         
-            SSLHandler handler = createSSLSessionHandler( nextFilter, session );
-
             if( SessionLog.isDebugEnabled( session ) )
             {
                 SessionLog.debug( session, " Filtered Write: " + handler );
@@ -567,8 +528,10 @@
     
     public void filterClose( NextFilter nextFilter, IoSession session, CloseFuture closeFuture
) throws SSLException
     {
+        SSLHandler handler = getSSLSessionHandler( session );
+
         WriteFuture future = null;
-        synchronized( session )
+        synchronized( handler )
         {
             try
             {
@@ -592,12 +555,7 @@
     private WriteFuture initiateClosure( NextFilter nextFilter, IoSession session ) throws
SSLException
     {
         SSLHandler handler = getSSLSessionHandler( session );
-        if( handler == null )
-        {
-            return WriteFuture.newNotWrittenFuture();
-        }
-        
-        // shut down
+        // if already shut down
         if( !handler.closeOutbound() )
         {
             return WriteFuture.newNotWrittenFuture();
@@ -608,7 +566,7 @@
         
         if( handler.isInboundDone() )
         {
-            releaseSSLSessionHandler( session );
+            handler.destroy();
         }
 
         if( session.containsAttribute( USE_NOTIFICATION ) )
@@ -636,21 +594,21 @@
         handleAppDataRead( nextFilter, handler );
     }
 
-    private void handleAppDataRead( NextFilter nextFilter, SSLHandler sslHandler )
+    private void handleAppDataRead( NextFilter nextFilter, SSLHandler handler )
     {
-        IoSession session = sslHandler.getSession();
-        if( !sslHandler.getAppBuffer().hasRemaining() )
+        IoSession session = handler.getSession();
+        if( !handler.getAppBuffer().hasRemaining() )
         {
             return;
         }
 
         if( SessionLog.isDebugEnabled( session ) )
         {
-            SessionLog.debug( session, " appBuffer: " + sslHandler.getAppBuffer() );
+            SessionLog.debug( session, " appBuffer: " + handler.getAppBuffer() );
         }
 
         // forward read app data
-        ByteBuffer readBuffer = SSLHandler.copy( sslHandler.getAppBuffer() );
+        ByteBuffer readBuffer = SSLHandler.copy( handler.getAppBuffer() );
         if( SessionLog.isDebugEnabled( session ) )
         {
             SessionLog.debug( session, " app data read: " + readBuffer + " (" + readBuffer.getHexDump()
+ ')' );
@@ -658,47 +616,18 @@
         nextFilter.messageReceived( session, readBuffer );
     }
 
-    // Utilities to mainpulate SSLHandler based on IoSession
-
-    private SSLHandler createSSLSessionHandler( NextFilter nextFilter, IoSession session
) throws SSLException
+    private SSLHandler getSSLSessionHandler( IoSession session )
     {
-        SSLHandler handler = getSSLSessionHandler( session );
+        SSLHandler handler = ( SSLHandler ) session.getAttribute( SSL_HANDLER );
         if( handler == null )
         {
-            boolean done = false;
-            try
-            {
-                handler =
-                    new SSLHandler( this, sslContext, session );
-                session.setAttribute( SSL_HANDLER, handler );
-                handler.handshake( nextFilter );
-                done = true;
-            }
-            finally 
-            {
-                if( !done )
-                {
-                    session.removeAttribute( SSL_HANDLER );
-                }
-            }
+            throw new IllegalStateException();
         }
-        
-        return handler;
-    }
-    
-    private SSLHandler getSSLSessionHandler( IoSession session )
-    {
-        return ( SSLHandler ) session.getAttribute( SSL_HANDLER );
-    }
-
-    private void releaseSSLSessionHandler( IoSession session )
-    {
-        SSLHandler sslHandler = getSSLSessionHandler( session );
-        if( sslHandler != null )
+        if( handler.getParent() != this )
         {
-            // release resources
-            sslHandler.release();
+            throw new IllegalArgumentException( "Not managed by this filter." );
         }
+        return handler;
     }
     
     /**

Modified: directory/network/trunk/src/java/org/apache/mina/filter/support/SSLHandler.java
URL: http://svn.apache.org/viewcvs/directory/network/trunk/src/java/org/apache/mina/filter/support/SSLHandler.java?rev=354680&r1=354679&r2=354680&view=diff
==============================================================================
--- directory/network/trunk/src/java/org/apache/mina/filter/support/SSLHandler.java (original)
+++ directory/network/trunk/src/java/org/apache/mina/filter/support/SSLHandler.java Tue Dec
 6 18:56:45 2005
@@ -49,9 +49,11 @@
 public class SSLHandler
 {
     private final SSLFilter parent;
+    private final SSLContext ctx;
     private final IoSession session;
     private final Queue scheduledWrites = new Queue();
-    private final SSLEngine sslEngine;
+
+    private SSLEngine sslEngine;
 
     /**
      * Encrypted data from the net
@@ -71,7 +73,7 @@
     /**
      * Empty buffer used during initial handshake and close operations
      */
-    private ByteBuffer hsBB = ByteBuffer.allocate( 0 );
+    private final ByteBuffer hsBB = ByteBuffer.allocate( 0 );
 
     /**
      * Handshake status
@@ -83,7 +85,7 @@
      */
     private boolean initialHandshakeComplete;
 
-    private boolean writingEncryptedData = false;
+    private boolean writingEncryptedData;
     
     /**
      * Constuctor.
@@ -95,7 +97,18 @@
     {
         this.parent = parent;
         this.session = session;
-        sslEngine = sslc.createSSLEngine();
+        this.ctx = sslc;
+        init();
+    }
+
+    public void init() throws SSLException
+    {
+        if( sslEngine != null )
+        {
+            return;
+        }
+
+        sslEngine = ctx.createSSLEngine();
         sslEngine.setUseClientMode( parent.isUseClientMode() );
 
         if ( parent.isWantClientAuth() )
@@ -130,8 +143,27 @@
         outNetBuffer = SSLByteBufferPool.getPacketBuffer();
         outNetBuffer.position( 0 );
         outNetBuffer.limit( 0 );
+        
+        writingEncryptedData = false;
     }
     
+    /**
+     * Release allocated ByteBuffers.
+     */
+    public void destroy()
+    {
+        if( sslEngine == null )
+        {
+            return;
+        }
+
+        SSLByteBufferPool.release( appBuffer );
+        SSLByteBufferPool.release( inNetBuffer );
+        SSLByteBufferPool.release( outNetBuffer );
+        scheduledWrites.clear();
+        sslEngine = null;
+    }
+
     public SSLFilter getParent()
     {
         return parent;
@@ -160,12 +192,12 @@
 
     public boolean isInboundDone()
     {
-        return sslEngine.isInboundDone();
+        return sslEngine == null || sslEngine.isInboundDone();
     }
 
     public boolean isOutboundDone()
     {
-        return sslEngine.isOutboundDone();
+        return sslEngine == null || sslEngine.isOutboundDone();
     }
 
     /**
@@ -322,7 +354,7 @@
      */
     public boolean closeOutbound() throws SSLException
     {
-        if( sslEngine.isOutboundDone() )
+        if( sslEngine == null || sslEngine.isOutboundDone() )
         {
             return false;
         }
@@ -339,16 +371,6 @@
         }
         outNetBuffer.flip();
         return true;
-    }
-
-    /**
-     * Release allocated ByteBuffers.
-     */
-    public void release()
-    {
-        SSLByteBufferPool.release( appBuffer );
-        SSLByteBufferPool.release( inNetBuffer );
-        SSLByteBufferPool.release( outNetBuffer );
     }
 
     /**



Mime
View raw message