cshannon commented on PR #2199: URL: https://github.com/apache/activemq/pull/2199#issuecomment-4925174479
> I suspect that we could have double-decrement `pendingSends` in XA. The `singlePendingSend()` uses <= 1 so negative values can pass the test. Neither `fireAfterCommit()` and `fireAfterRollback()` has a guard preventing both from running on the same sync list. That's OK, and we can always improve in a follow-up PR. With the current design, I am not sure if there is a great way to guarantee we don't miss a decrement or double decrement using a simple counter like it is doing now. The problem is there is so many code paths for the decrement (async etc) and possible error scenarios it would be hard to ensure it's correct. I think a future PR to refactor to just try and simplify and reduce the number of code paths where increment/decrement happen would help provide better guarantees it's correct. One way to handle the double decrement case would be to to track pendingSend message Ids in a Set instead of using a counter and just remove instead of decrement. However, that doesn't help missing a decrement and would cause a memory leak so not sure that makes sense either. -- 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
