cshannon commented on PR #1659: URL: https://github.com/apache/activemq/pull/1659#issuecomment-3891444133
The latest changes here still do not address this comment I made: https://github.com/apache/activemq/pull/1659#pullrequestreview-3786189384 This is still marshaling which we should not be doing, with the VM transport we should just be using the message copy methods. BUT, the question is still what is the real issue and why is this happening? As pointed out, **we already copy the message on dispatch to consumers**. The test you wrote does fail without your changes but ONLY because you are only comparing the String instances to see if they are the same, which they won't be as you are using OpenWire. However, with calling copy() the String instances are the same as Strings are immutable. If you modified your test to compare instance equality for the TextMessage instances and not just the bodies the test would pass because we copy on dispatch. My guess is the real issue could be a race condition during the copy or when the body is converted between states. The messages themselves are generally not thread safe which is why we copy on dispatch so each consumer gets their own copy, but some messages can toggle state..they can convert transparently between marshaled body and the in memory usable body (same with message properties). @pradeep85841 - Are you only using Text messages? So for a text message, the message will either store the body as a String or it will store it as a buffer and can switch. My guess is things are breaking during that switch as multiple threads are probably calling some of these methods concurrently: https://github.com/apache/activemq/blob/649aa0ae679ec104961408b4a1363ca88d0916bd/activemq-client/src/main/java/org/apache/activemq/command/ActiveMQTextMessage.java#L73-L181 This might apply to other message types as well but this was the most noticeable type because it tries to only store either the String or byte buffer to save space. Going back a long time ago, I actually made some changes here to try and prevent issues without adding synchronization but it's not perfect: https://issues.apache.org/jira/browse/AMQ-5857 So we just need to figure out the exact issue, if it's the copy() that is being done on dispatch (maybe multiple connections are calling copy at the same time on dispatch) we may just need to add some sort of synchronization there. But we need ot identify the real cause, a good first step might be to just test what happens if you apply the "synchronized" key word on the copy() method or also on the other methods that due mutations like getText(), setText(), storeContent() etc and re-run things to see if the issue is gone to help narrow it down to that. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected] For further information, visit: https://activemq.apache.org/contact
