Description of problem: TransactionContextImpl should call fireCommitEvent() within the try block, not the finally block -- as it stands in the 5.2 codebase, if fireBeforeCommitEvent() throws an exception, that exception is lost since fireCommitEvent() throws an assertion exception due to the transaction not cleaning up properly. changelist 35501 fixes this on the london 5.2 codebase. It's not relevant for 6.0, as this code has been substantially refactored and the problem no longer exists there. Version-Release number of selected component (if applicable): 5.2 How reproducible: Only reproduced if fireCommitEvent throws an exception. Steps to Reproduce: 1. 2. 3. Actual results: Expected results: Additional info:
The semantics around disconnected data objects could be violated by this change in certain cases, so we're going to need to think about this change a bit, and also see how it works out on the London tree.
Yes, it wasn't clear to me exactly where the disconnectDataObject call should go after moving the fireCommitEvent() call. The intention of the change is to prevent fireCommitEvent from being called after an exception is thrown (which is what the troika code already does). But this code has been refactored so much for troika that I couldn't just grab it as-is for 5.2
Change 35501 integrated to 5.2.x at 35689.