Recently, I was looking at the code of Transaction and found that TransactionSynchronizationUtils is too rude to execute user callback code, and all the exceptions that may be thrown in the callback method are directly swallowed.

org.springframework.transaction.support.TransactionSynchronizationUtils#triggerBeforeCompletion
public static void triggerBeforeCompletion() {
        for (TransactionSynchronization synchronization : TransactionSynchronizationManager.getSynchronizations()) {
            try {
                synchronization.beforeCompletion();
            }
            catch (Throwable ex) { //  Swallow all Throwable，but ThreadDeath/VirtualMachineError should throw.
                logger.error("TransactionSynchronization.beforeCompletion threw exception", ex);
            }
        }
    }
org.springframework.transaction.support.TransactionSynchronizationUtils#invokeAfterCompletion
public static void invokeAfterCompletion(@Nullable List<TransactionSynchronization> synchronizations,
            int completionStatus) {

        if (synchronizations != null) {
            for (TransactionSynchronization synchronization : synchronizations) {
                try {
                    synchronization.afterCompletion(completionStatus);
                }
                catch (Throwable ex) {
                    logger.error("TransactionSynchronization.afterCompletion threw exception", ex);
                }
            }
        }
    }

Strategy used in Tomcat

public static void handleThrowable(Throwable t) {
        if (t instanceof ThreadDeath) {
            throw (ThreadDeath) t;
        }
        if (t instanceof StackOverflowError) {
            // Swallow silently - it should be recoverable
            return;
        }
        if (t instanceof VirtualMachineError) {
            throw (VirtualMachineError) t;
        }
        // All other instances of Throwable will be silently swallowed
    }

Comment From: PerJoker

Thank you for your reply. Should I modify it and submit it to merge? I am a bit unclear about the process

Comment From: PerJoker

@sbrannen

Comment From: sbrannen

@PerJoker, are you suggesting that exceptions should not be swallowed if they are of type ThreadDeath, StackOverflowError, or VirtualMachineError?

Or are you proposing something else?

Comment From: sbrannen

Should I modify it and submit it to merge? I am a bit unclear about the process

The status: waiting-for-triage label on the right-hand side of this page indicates that the team has not yet assessed this issue.

Comment From: PerJoker

@PerJoker, are you suggesting that exceptions should not be swallowed if they are of type ThreadDeath, StackOverflowError, or VirtualMachineError?

Or are you proposing something else?

Yes , the types of ThreadDeath, StackOverflowError, or VirtualMachineError should be thrown always.

Comment From: PerJoker

@sbrannen Can I should merge？

Comment From: snicoll

Thanks for the suggestion but we believe that the code is fine as it is. With beforeCompletion, it's part of exception recovery when something fails on transaction completion. We catch and log unexpected follow-up exceptions there so that the rest of the cleanup continues - and in particular so that the original exception is not being overridden.