Skip to content

[fix] Restore interrupt status in InterruptedException catch blocks - #26710

Open
TimurRakhmatullin86 wants to merge 1 commit into
apache:masterfrom
TimurRakhmatullin86:fix/restore-interrupt-status
Open

TimurRakhmatullin86 wants to merge 1 commit into
apache:masterfrom
TimurRakhmatullin86:fix/restore-interrupt-status

Conversation

@TimurRakhmatullin86

Copy link
Copy Markdown
Contributor

Motivation

When Java code catches InterruptedException, the JVM automatically clears the interrupt flag on the current thread. If the catch block does not restore the flag by calling Thread.currentThread().interrupt(), all upstream callers lose visibility into the fact that an interrupt was requested. This is a well-known correctness issue documented in Java Concurrency in Practice (section 7.1.2) and the InterruptedException Javadoc.

In a system like Pulsar — where graceful shutdown, topic unloading, and thread pool termination rely on cooperative interrupt signaling — swallowing the interrupt flag can cause:

  • Shutdown hangs: threads that should terminate keep running because they never see the interrupt.
  • Resource leaks: cleanup code gated on Thread.interrupted() is silently skipped.
  • Silent failures: operations that should abort continue with stale or inconsistent state.

Modifications

This PR adds Thread.currentThread().interrupt() to 35 catch blocks across 31 files in the following modules:

Module Files
pulsar-broker (service) BrokerService, PersistentTopic, PersistentSubscription, NonPersistentTopic
pulsar-broker (loadbalance) ServiceUnitStateChannelImpl, TransferShedder, BrokerRegistryImpl
pulsar-broker (namespace) OwnedBundle, OwnershipCache
pulsar-broker (admin/web) PulsarWebResource, WebService, PackagesBase, NonPersistentTopics, LoadReportCommand
pulsar-broker (compaction) StrategicTwoPhaseCompactor
pulsar-broker (other) BookKeeperClientFactoryImpl, PulsarClusterMetadataTeardown, LocalBookkeeperEnsemble
pulsar-broker-common AuthorizationService, OneStageAuthenticationState, BookieRackAffinityMapping
pulsar-common FileUtils, FutureUtil
managed-ledger ManagedLedgerImpl
pulsar-metadata PulsarRegistrationManager, PulsarLedgerUnderreplicationManager, PulsarLedgerAuditorManager, ZKSessionWatcher, MetadataStoreTableViewImpl
pulsar-proxy ProxyStats
pulsar-transaction MLTransactionLogImpl

For catch blocks that handle only InterruptedException, the fix is:

} catch (InterruptedException e) {
    Thread.currentThread().interrupt();
    // existing handling ...
}

For multi-catch blocks (e.g. catch (InterruptedException | ExecutionException e)), the fix is:

} catch (InterruptedException | ExecutionException e) {
    if (e instanceof InterruptedException) {
        Thread.currentThread().interrupt();
    }
    // existing handling ...
}

Verifying this change

This is a mechanical, behavior-preserving fix. Each change adds exactly one Thread.currentThread().interrupt() call at the start of a catch block; no control flow or logic is altered.

  • Existing unit and integration tests continue to pass.
  • The fix follows the pattern already used in many other places in the Pulsar codebase (e.g., PulsarService.java, ExtensibleLoadManagerImpl.java, TopicTransactionBuffer.java).

Documentation

  • doc-not-needed — No documentation changes needed for a bug fix that preserves existing behavior.

🤖 Generated with Claude Code

When catching InterruptedException, the interrupt flag on the current thread
is cleared by the JVM. Code that catches this exception must restore the flag
by calling Thread.currentThread().interrupt() before re-throwing or returning,
otherwise upstream code (and the thread's own interrupt-driven shutdown logic)
cannot detect that an interrupt occurred. This is a well-documented Java best
practice (see Java Concurrency in Practice, section 7.1.2, and the
java.lang.InterruptedException Javadoc).

This commit fixes 35 catch blocks across 31 files in the following modules
where the interrupt status was silently swallowed:

- pulsar-broker (service, loadbalance, namespace, admin, web, compaction,
  transaction, tools, zookeeper)
- pulsar-broker-common (authorization, authentication, bookie)
- pulsar-common (FileUtils, FutureUtil)
- managed-ledger (ManagedLedgerImpl)
- pulsar-metadata (bookkeeper integration, ZKSessionWatcher, TableView)
- pulsar-proxy (ProxyStats)
- pulsar-transaction (MLTransactionLogImpl)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant