You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@activemq.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2023/04/05 10:35:00 UTC

[jira] [Work logged] (ARTEMIS-4235) Losing bridge connection when sending map message.

     [ https://issues.apache.org/jira/browse/ARTEMIS-4235?focusedWorklogId=855014&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-855014 ]

ASF GitHub Bot logged work on ARTEMIS-4235:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 05/Apr/23 10:34
            Start Date: 05/Apr/23 10:34
    Worklog Time Spent: 10m 
      Work Description: gemmellr commented on code in PR #4426:
URL: https://github.com/apache/activemq-artemis/pull/4426#discussion_r1158316804


##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/multiprotocol/JMSMessageConsumerTest.java:
##########
@@ -193,4 +195,66 @@ private void testDurableSubscriptionWithConfigurationManagedQueue(ConnectionSupp
          assertEquals("color = 'BLUE'", queue.getFilter().getFilterString().toString());
       }
    }
+
+   @Test(timeout = 30000)
+   public void testMapMessageConversionBetweenOpenWireAndAMQP() throws Exception {
+      testMapMessageConversion(createOpenWireConnection(), createConnection());
+   }
+
+   @Test(timeout = 30000)
+   public void testMapMessageConversionBetweenAMQPAndOpenWire() throws Exception {
+      testMapMessageConversion(createConnection(), createOpenWireConnection());
+   }
+
+   @Test(timeout = 30000)
+   public void testMapMessageConversionBetweenCoreAndAMQP() throws Exception {
+      testMapMessageConversion(createCoreConnection(), createConnection());
+   }
+
+   @Test(timeout = 30000)
+   public void testMapMessageConversionBetweenAMQPAndCore() throws Exception {
+      testMapMessageConversion(createConnection(), createCoreConnection());
+   }
+
+   @Test(timeout = 30000)
+   public void testMapMessageConversionBetweenCoreAndOpenWire() throws Exception {
+      testMapMessageConversion(createCoreConnection(), createOpenWireConnection());
+   }
+
+   @Test(timeout = 30000)
+   public void testMapMessageConversionBetweenOpenWireAndCore() throws Exception {
+      testMapMessageConversion(createOpenWireConnection(), createCoreConnection());
+   }
+
+   private void testMapMessageConversion(Connection sender, Connection consumer) throws Exception {
+      final String NAME = "myPropertyName";
+      final String VALUE = RandomUtil.randomString();
+
+      try {
+         Session session1 = sender.createSession(false, Session.AUTO_ACKNOWLEDGE);
+         Session session2 = consumer.createSession(false, Session.AUTO_ACKNOWLEDGE);
+
+         javax.jms.Queue queue1 = session1.createQueue(getQueueName());
+         javax.jms.Queue queue2 = session2.createQueue(getQueueName());

Review Comment:
   Should be able to use the same destination object for both, but if wanting to avoid it then just inlining the creations might be neater given the variables arent reused, avoiding them giving impression there are 'two queues' when its just one.



##########
artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireMessageConverter.java:
##########
@@ -111,9 +111,13 @@ public static org.apache.activemq.artemis.api.core.Message inbound(final Message
       final ActiveMQBuffer body = coreMessage.getBodyBuffer();
 
       final ByteSequence contents = messageSend.getContent();
-      if (contents == null && coreType == org.apache.activemq.artemis.api.core.Message.TEXT_TYPE) {
-         body.writeNullableString(null);
-      } else if (contents != null) {
+      if (contents == null) {
+         if (coreType == org.apache.activemq.artemis.api.core.Message.TEXT_TYPE) {
+            body.writeNullableString(null);
+         } else if (coreType == org.apache.activemq.artemis.api.core.Message.MAP_TYPE) {
+            body.writeByte(DataConstants.NULL);
+         }

Review Comment:
   The new tests dont appear to cover this case, the main addition of the PR and how the issue was found?
   
   Also feels like this could be unit tested rather than system tested.



##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/multiprotocol/JMSMessageConsumerTest.java:
##########
@@ -193,4 +195,66 @@ private void testDurableSubscriptionWithConfigurationManagedQueue(ConnectionSupp
          assertEquals("color = 'BLUE'", queue.getFilter().getFilterString().toString());
       }
    }
+
+   @Test(timeout = 30000)
+   public void testMapMessageConversionBetweenOpenWireAndAMQP() throws Exception {
+      testMapMessageConversion(createOpenWireConnection(), createConnection());
+   }
+
+   @Test(timeout = 30000)
+   public void testMapMessageConversionBetweenAMQPAndOpenWire() throws Exception {
+      testMapMessageConversion(createConnection(), createOpenWireConnection());
+   }
+
+   @Test(timeout = 30000)
+   public void testMapMessageConversionBetweenCoreAndAMQP() throws Exception {
+      testMapMessageConversion(createCoreConnection(), createConnection());
+   }
+
+   @Test(timeout = 30000)
+   public void testMapMessageConversionBetweenAMQPAndCore() throws Exception {
+      testMapMessageConversion(createConnection(), createCoreConnection());
+   }
+
+   @Test(timeout = 30000)
+   public void testMapMessageConversionBetweenCoreAndOpenWire() throws Exception {
+      testMapMessageConversion(createCoreConnection(), createOpenWireConnection());
+   }
+
+   @Test(timeout = 30000)
+   public void testMapMessageConversionBetweenOpenWireAndCore() throws Exception {
+      testMapMessageConversion(createOpenWireConnection(), createCoreConnection());
+   }
+
+   private void testMapMessageConversion(Connection sender, Connection consumer) throws Exception {
+      final String NAME = "myPropertyName";
+      final String VALUE = RandomUtil.randomString();
+
+      try {
+         Session session1 = sender.createSession(false, Session.AUTO_ACKNOWLEDGE);
+         Session session2 = consumer.createSession(false, Session.AUTO_ACKNOWLEDGE);
+
+         javax.jms.Queue queue1 = session1.createQueue(getQueueName());
+         javax.jms.Queue queue2 = session2.createQueue(getQueueName());
+
+         final MessageConsumer consumer2 = session2.createConsumer(queue2);
+
+         MessageProducer producer = session1.createProducer(queue1);
+         producer.setDeliveryMode(DeliveryMode.PERSISTENT);
+         sender.start();
+
+         MapMessage message = session1.createMapMessage();
+         message.setStringProperty(NAME, VALUE);
+         producer.send(message);
+
+         Message received = consumer2.receive(100);

Review Comment:
   100 seems a little low for a message we do expect to receive, could be unreliable in slow CI envs.



##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/multiprotocol/JMSMessageConsumerTest.java:
##########
@@ -193,4 +195,66 @@ private void testDurableSubscriptionWithConfigurationManagedQueue(ConnectionSupp
          assertEquals("color = 'BLUE'", queue.getFilter().getFilterString().toString());
       }
    }
+
+   @Test(timeout = 30000)
+   public void testMapMessageConversionBetweenOpenWireAndAMQP() throws Exception {
+      testMapMessageConversion(createOpenWireConnection(), createConnection());
+   }
+
+   @Test(timeout = 30000)
+   public void testMapMessageConversionBetweenAMQPAndOpenWire() throws Exception {
+      testMapMessageConversion(createConnection(), createOpenWireConnection());
+   }
+
+   @Test(timeout = 30000)
+   public void testMapMessageConversionBetweenCoreAndAMQP() throws Exception {
+      testMapMessageConversion(createCoreConnection(), createConnection());
+   }
+
+   @Test(timeout = 30000)
+   public void testMapMessageConversionBetweenAMQPAndCore() throws Exception {
+      testMapMessageConversion(createConnection(), createCoreConnection());
+   }
+
+   @Test(timeout = 30000)
+   public void testMapMessageConversionBetweenCoreAndOpenWire() throws Exception {
+      testMapMessageConversion(createCoreConnection(), createOpenWireConnection());
+   }
+
+   @Test(timeout = 30000)
+   public void testMapMessageConversionBetweenOpenWireAndCore() throws Exception {
+      testMapMessageConversion(createOpenWireConnection(), createCoreConnection());
+   }
+
+   private void testMapMessageConversion(Connection sender, Connection consumer) throws Exception {

Review Comment:
   Something like senderConnection and consumerConnection would seem more descriptive...you could then follow through with 'sender[Foo]' and 'consumer[Foo]' convention for related objects and wouldnt need to call the sole consumer "consumer2".





Issue Time Tracking
-------------------

            Worklog Id:     (was: 855014)
    Remaining Estimate: 0h
            Time Spent: 10m

> Losing bridge connection when sending map message.
> --------------------------------------------------
>
>                 Key: ARTEMIS-4235
>                 URL: https://issues.apache.org/jira/browse/ARTEMIS-4235
>             Project: ActiveMQ Artemis
>          Issue Type: Bug
>    Affects Versions: 2.27.0
>         Environment: Windows 10
>            Reporter: Peter Sunnerdahl
>            Priority: Critical
>         Attachments: TestMQ.java, broker.xml, testMQMap.xml
>
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> Artemis 2.27.0 installations with a bridge configured. When sending a map message with an empty map the message is sent to DLQ and the connection is lost. It will not recover unless I delete the message and restarts the service. Same testcase works on old ActiveMQ. Found this when upgrading.
> {noformat}
> 2023-04-03 14:17:59,339 WARN  [org.apache.activemq.artemis.protocol.amqp.logger] AMQ111005: Failed to convert message. Sending it to Dead Letter Address.
> org.apache.activemq.artemis.protocol.amqp.converter.coreWrapper.ConversionException: readerIndex(0) + length(1) exceeds writerIndex(0): ReadOnlyByteBuf(ridx: 0, widx: 0, cap: 0/0, unwrapped: UnpooledSlicedByteBuf(ridx: 0, widx: 0, cap: 0/0, unwrapped: UnpooledByteBufAllocator$InstrumentedUnpooledUnsafeHeapByteBuf(ridx: 0, widx: 1096, cap: 1096)))
>     at org.apache.activemq.artemis.protocol.amqp.converter.CoreAmqpConverter.fromCore(CoreAmqpConverter.java:319) ~[artemis-amqp-protocol-2.27.0.jar:2.27.0]
>     at org.apache.activemq.artemis.protocol.amqp.converter.CoreAmqpConverter.checkAMQP(CoreAmqpConverter.java:80) ~[artemis-amqp-protocol-2.27.0.jar:2.27.0]
>     at org.apache.activemq.artemis.protocol.amqp.proton.ProtonServerSenderContext.executeDelivery(ProtonServerSenderContext.java:541) ~[artemis-amqp-protocol-2.27.0.jar:2.27.0]
>     at org.apache.activemq.artemis.core.server.impl.MessageReferenceImpl.run(MessageReferenceImpl.java:134) ~[artemis-server-2.27.0.jar:2.27.0]
>     at io.netty.util.concurrent.AbstractEventExecutor.runTask(AbstractEventExecutor.java:174) ~[netty-common-4.1.82.Final.jar:4.1.82.Final]
>     at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:167) ~[netty-common-4.1.82.Final.jar:4.1.82.Final]
>     at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:470) ~[netty-common-4.1.82.Final.jar:4.1.82.Final]
>     at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:569) ~[netty-transport-4.1.82.Final.jar:4.1.82.Final]
>     at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997) ~[netty-common-4.1.82.Final.jar:4.1.82.Final]
>     at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74) ~[netty-common-4.1.82.Final.jar:4.1.82.Final]
>     at org.apache.activemq.artemis.utils.ActiveMQThreadFactory$1.run(ActiveMQThreadFactory.java:118) ~[artemis-commons-2.27.0.jar:?]
> Caused by: java.lang.IndexOutOfBoundsException: readerIndex(0) + length(1) exceeds writerIndex(0): ReadOnlyByteBuf(ridx: 0, widx: 0, cap: 0/0, unwrapped: UnpooledSlicedByteBuf(ridx: 0, widx: 0, cap: 0/0, unwrapped: UnpooledByteBufAllocator$InstrumentedUnpooledUnsafeHeapByteBuf(ridx: 0, widx: 1096, cap: 1096)))
>     at io.netty.buffer.AbstractByteBuf.checkReadableBytes0(AbstractByteBuf.java:1442) ~[netty-buffer-4.1.82.Final.jar:4.1.82.Final]
>     at io.netty.buffer.AbstractByteBuf.readByte(AbstractByteBuf.java:730) ~[netty-buffer-4.1.82.Final.jar:4.1.82.Final]
>     at io.netty.buffer.WrappedByteBuf.readByte(WrappedByteBuf.java:529) ~[netty-buffer-4.1.82.Final.jar:4.1.82.Final]
>     at org.apache.activemq.artemis.utils.collections.TypedProperties.decode(TypedProperties.java:464) ~[artemis-commons-2.27.0.jar:?]
>     at org.apache.activemq.artemis.utils.collections.TypedProperties.decode(TypedProperties.java:547) ~[artemis-commons-2.27.0.jar:?]
>     at org.apache.activemq.artemis.reader.MapMessageUtil.readBodyMap(MapMessageUtil.java:46) ~[artemis-core-client-2.27.0.jar:2.27.0]
>     at org.apache.activemq.artemis.protocol.amqp.converter.coreWrapper.CoreMapMessageWrapper.decode(CoreMapMessageWrapper.java:222) ~[artemis-amqp-protocol-2.27.0.jar:2.27.0]
>     at org.apache.activemq.artemis.protocol.amqp.converter.CoreAmqpConverter.fromCore(CoreAmqpConverter.java:98) ~[artemis-amqp-protocol-2.27.0.jar:2.27.0]{noformat}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)