fix: handle InterruptedException properly in ProgressBar.close() #193
Open
rubaiyat-bsse wants to merge 1 commit into
Open
fix: handle InterruptedException properly in ProgressBar.close() #193rubaiyat-bsse wants to merge 1 commit into
rubaiyat-bsse wants to merge 1 commit into
Conversation
The close() method silently swallowed both InterruptedException and
ExecutionException with an empty catch block (/* NOOP */), which was
flagged by PMD as an EmptyCatchBlock violation (Error Prone, priority 3).
Split the combined catch block into two separate handlers:
- InterruptedException: calls Thread.currentThread().interrupt() to
restore the interrupt flag, following Java best practice. Swallowing
this exception without restoring the flag discards thread interruption
state, making it invisible to the calling thread.
- ExecutionException: acknowledged with an explanatory comment. A failure
in the final render is genuinely non-critical since the progress bar is
already in the process of closing.
Functionality is unchanged — all existing tests pass.
rubaiyat-bsse
force-pushed
the
refactoring-contribution
branch
from
July 2, 2026 20:22
2c77256 to
8ac731a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The
close()method inProgressBarwas silently swallowingInterruptedExceptionandExecutionExceptionwith an empty catch block (/* NOOP */). This was flagged by PMD as anEmptyCatchBlockviolation (Error Prone category, priority 3)Problem
Catching
InterruptedExceptionwithout restoring the interrupt flag discards the thread's interrupted state silently. Any code up the call stack that checksThread.isInterrupted()will never know the interruption happened, which can cause subtle and hard-to-debug issues in multi-threaded environments.Changes
catchinto two separate handlersInterruptedException→ callsThread.currentThread().interrupt()to restore the interrupt flag per Java best practiceExecutionException→ retained as a no-op but with an explanatory comment, since a failed final render is non-critical during shutdownTesting
All existing tests pass. Behaviour is functionally unchanged.