qpid-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From oru...@apache.org
Subject svn commit: r1669612 - in /qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src: main/java/org/apache/qpid/server/protocol/v0_8/AMQProtocolEngine.java test/java/org/apache/qpid/server/protocol/v0_8/AMQProtocolEngineTest.java
Date Fri, 27 Mar 2015 15:50:50 GMT
Author: orudyy
Date: Fri Mar 27 15:50:50 2015
New Revision: 1669612

URL: http://svn.apache.org/r1669612
Log:
QPID-6469: Improve AMQProtocolEngine#exception to guard against an exception whilst trying
to send response to client from the method and re-throw unexpected exception

Modified:
    qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQProtocolEngine.java
    qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQProtocolEngineTest.java

Modified: qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQProtocolEngine.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQProtocolEngine.java?rev=1669612&r1=1669611&r2=1669612&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQProtocolEngine.java
(original)
+++ qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQProtocolEngine.java
Fri Mar 27 15:50:50 2015
@@ -42,7 +42,6 @@ import java.util.concurrent.ConcurrentHa
 import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
 
 import javax.security.auth.Subject;
@@ -1161,8 +1160,7 @@ public class AMQProtocolEngine implement
     {
         if (throwable instanceof AMQProtocolHeaderException)
         {
-            writeFrame(new ProtocolInitiation(ProtocolVersion.getLatestSupportedVersion()));
-            _sender.close();
+            sendResponseAndCloseSender(new ProtocolInitiation(ProtocolVersion.getLatestSupportedVersion()));
 
             _logger.error("Error in protocol initiation " + this + ":" + getRemoteAddress()
+ " :" + throwable.getMessage(), throwable);
         }
@@ -1181,30 +1179,57 @@ public class AMQProtocolEngine implement
                                                                                         
            throwable.getMessage()),
                                                                                         
    _currentClassId,
                                                                                         
    _currentMethodId);
-
-                try
+                sendResponseAndCloseSender(closeBody.generateFrame(0));
+            }
+            finally
+            {
+                if (!(throwable instanceof TransportException
+                        || throwable instanceof ConnectionScopedRuntimeException))
                 {
-                    writeFrame(closeBody.generateFrame(0));
+                    if (throwable instanceof Error)
+                    {
+                        throw (Error) throwable;
+                    }
 
-                    _sender.close();
-                }
-                catch(SenderException e)
-                {
-                    // ignore
+                    if (throwable instanceof RuntimeException)
+                    {
+                        throw (RuntimeException) throwable;
+                    }
+
+                    if (throwable instanceof Throwable)
+                    {
+                        throw new ServerScopedRuntimeException("Unexpected exception", throwable);
+                    }
                 }
+            }
+        }
+    }
 
+    private void sendResponseAndCloseSender(AMQDataBlock dataBlock)
+    {
+        try
+        {
+            writeFrame(dataBlock);
+        }
+        catch(SenderException e)
+        {
+            if (_logger.isDebugEnabled())
+            {
+                _logger.debug("Exception occurred on sending response", e);
             }
-            finally
+        }
+        finally
+        {
+            try
             {
-                if(throwable instanceof Error)
-                {
-                    throw (Error) throwable;
-                }
-                if(throwable instanceof ServerScopedRuntimeException)
+                _sender.close();
+            }
+            catch(SenderException e)
+            {
+                if (_logger.isDebugEnabled())
                 {
-                    throw (ServerScopedRuntimeException) throwable;
+                    _logger.debug("Exception occurred on sender close", e);
                 }
-
             }
         }
     }

Modified: qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQProtocolEngineTest.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQProtocolEngineTest.java?rev=1669612&r1=1669611&r2=1669612&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQProtocolEngineTest.java
(original)
+++ qpid/trunk/qpid/java/broker-plugins/amqp-0-8-protocol/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQProtocolEngineTest.java
Fri Mar 27 15:50:50 2015
@@ -20,20 +20,30 @@
  */
 package org.apache.qpid.server.protocol.v0_8;
 
+import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
+import java.nio.ByteBuffer;
 import java.util.HashMap;
 import java.util.Map;
 
+import org.apache.qpid.AMQException;
+import org.apache.qpid.framing.AMQFrameDecodingException;
 import org.apache.qpid.framing.FieldTable;
 import org.apache.qpid.properties.ConnectionStartProperties;
+import org.apache.qpid.protocol.AMQConstant;
 import org.apache.qpid.server.model.Broker;
 import org.apache.qpid.server.model.Transport;
 import org.apache.qpid.server.model.port.AmqpPort;
 import org.apache.qpid.server.util.BrokerTestHelper;
+import org.apache.qpid.server.util.ConnectionScopedRuntimeException;
+import org.apache.qpid.server.util.ServerScopedRuntimeException;
 import org.apache.qpid.test.utils.QpidTestCase;
+import org.apache.qpid.transport.ByteBufferSender;
+import org.apache.qpid.transport.SenderException;
 import org.apache.qpid.transport.network.NetworkConnection;
 
 public class AMQProtocolEngineTest extends QpidTestCase
@@ -92,4 +102,103 @@ public class AMQProtocolEngineTest exten
 
         assertFalse("Unexpected closeWhenNoRoute after client properties set", engine.isCloseWhenNoRoute());
     }
+
+    public void testThrownExceptionOnSendingResponseFromExceptionHandler()
+    {
+        ByteBufferSender sender = mock(ByteBufferSender.class);
+        when(_network.getSender()).thenReturn(sender);
+        doThrow(new SenderException("exception on close")).when(sender).close();
+        doThrow(new SenderException("exception on send")).when(sender).send(any(ByteBuffer.class));
+
+        AMQProtocolEngine engine = new AMQProtocolEngine(_broker, _network, 0, _port, _transport);
+
+        try
+        {
+            engine.exception(new ConnectionScopedRuntimeException("test"));
+        }
+        catch (Exception e)
+        {
+            fail("Unexpected exception is thrown " + e);
+        }
+
+        doThrow(new NullPointerException("unexpected exception")).when(sender).send(any(ByteBuffer.class));
+        try
+        {
+            engine.exception(new ConnectionScopedRuntimeException("test"));
+            fail("Unexpected exception should be reported");
+        }
+        catch (NullPointerException e)
+        {
+            // pass
+        }
+
+    }
+
+    public void testExceptionHandling()
+    {
+        ByteBufferSender sender = mock(ByteBufferSender.class);
+        when(_network.getSender()).thenReturn(sender);
+
+        AMQProtocolEngine engine = new AMQProtocolEngine(_broker, _network, 0, _port, _transport);
+
+        try
+        {
+            engine.exception(new ConnectionScopedRuntimeException("test"));
+        }
+        catch (Exception e)
+        {
+            fail("Unexpected exception is thrown " + e);
+        }
+
+
+        try
+        {
+            engine.exception(new SenderException("test"));
+        }
+        catch (NullPointerException e)
+        {
+            fail("Unexpected exception should be reported");
+        }
+
+        try
+        {
+            engine.exception(new NullPointerException("test"));
+            fail("NullPointerException should be re-thrown");
+        }
+        catch (NullPointerException e)
+        {
+            //pass
+        }
+
+        try
+        {
+            engine.exception(new ServerScopedRuntimeException("test"));
+            fail("ServerScopedRuntimeException should be re-thrown");
+        }
+        catch (ServerScopedRuntimeException e)
+        {
+            //pass
+        }
+
+        try
+        {
+            engine.exception(new AMQException(AMQConstant.INTERNAL_ERROR, "test"));
+            fail("AMQException should be re-thrown as ServerScopedRuntimeException");
+        }
+        catch (ServerScopedRuntimeException e)
+        {
+            //pass
+        }
+
+        try
+        {
+            engine.exception(new Error("test"));
+            fail("Error should be re-thrown");
+        }
+        catch (Error e)
+        {
+            //pass
+        }
+    }
+
 }



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org


Mime
View raw message