pradeep85841 commented on PR #1659: URL: https://github.com/apache/activemq/pull/1659#issuecomment-3912506450
> i think adding the storeContent() part to the clearUnmarshalledState() is ok but I'm curious where/why we need to do it. The only spot I could find it being called is when reduceMemoryFootprint is true and we are validating that it's marshaled before calling it. > > I just want to make sure we fully understand the root cause of your issue so that we can validate the fix is correct. Hi @cshannon, Great question. After digging into the analysis from @tabish121 and my test results, here is the 'why': The root cause is a state-transition gap. Currently, clearUnmarshalledState() assumes the content bytes are already there and simply wipes the 'live' data (String, Map, etc.). If a background task (like a memory sweep) hits this method before marshaling is finished, the message becomes empty—leading to the data loss I caught. By adding a quick storeContent() check inside clearUnmarshalledState(), we make the message 'self-healing.' It ensures that the message protects its own data integrity regardless of the timing or threading context. I’ve confirmed that this logic fix works perfectly without needing synchronized locks. Since this vulnerability exists across MapMessage, ObjectMessage, and StreamMessage as well, I’d like to apply this hardening to all of them for consistency. -- 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
