gnodet-bot commented on code in PR #26871:
URL: https://github.com/apache/camel/pull/26871#discussion_r4227192614
##########
components/camel-tahu/src/main/java/org/apache/camel/component/tahu/handlers/TahuEdgeClient.java:
##########
@@ -107,6 +107,7 @@ public void shutdown() {
} catch (TimeoutException te) {
clientSubmittedFuture.cancel(true);
} catch (CancellationException | ExecutionException |
InterruptedException e) {
+ Thread.currentThread().interrupt();
Review Comment:
⚠️ **Bug: `interrupt()` called for non-`InterruptedException` cases**
The multi-catch `CancellationException | ExecutionException |
InterruptedException` lumps together three unrelated exception types.
`Thread.currentThread().interrupt()` is only semantically correct for
`InterruptedException` — calling it when a `CancellationException` or
`ExecutionException` is thrown spuriously sets the interrupt flag, which will
break any subsequent blocking call in the caller.
Split the catch block:
```suggestion
} catch (CancellationException | ExecutionException e) {
LOG.warn(loggingMarker, "Caught exception waiting for client
shutdown", e);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
LOG.warn(loggingMarker, "Caught exception waiting for client
shutdown", e);
```
##########
dsl/camel-jbang/camel-jbang-plugin-kubernetes/src/main/java/org/apache/camel/dsl/jbang/core/commands/kubernetes/KubernetesPodLogs.java:
##########
@@ -152,6 +152,7 @@ private void sleepWell() {
try {
Thread.sleep(2000L);
} catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
printer().printf("Interrupted while waiting for pod - %s%n",
e.getMessage());
}
}
Review Comment:
⚠️ **Interrupt flag restored but method returns normally — caller will spin**
`sleepWell()` is called in a retry loop. After restoring the interrupt flag,
the method falls through the `catch` block, prints, and returns normally. The
caller then re-invokes `sleepWell()` on the next iteration, which calls
`Thread.sleep()` — and `Thread.sleep()` will immediately throw
`InterruptedException` again because the flag is still set, producing an
infinite fast loop.
The fix should propagate the interrupt signal so the caller can exit:
```suggestion
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
printer().printf("Interrupted while waiting for pod - %s%n",
e.getMessage());
return; // exit the loop
```
(Or restructure the caller loop to check
`Thread.currentThread().isInterrupted()` and break.)
##########
components/camel-smpp/src/main/java/org/apache/camel/component/smpp/MessageReceiverListenerImpl.java:
##########
@@ -80,7 +80,7 @@ public void onAcceptAlertNotification(AlertNotification
alertNotification) {
// wait for initialization in case consumer is active before
startup listener above has been executed
initialized.await(10, TimeUnit.SECONDS);
} catch (InterruptedException e) {
- // ignore
+ Thread.currentThread().interrupt();
}
LOG.debug("Received an alertNotification {}", alertNotification);
Review Comment:
⚠️ **Interrupt flag restored but execution falls through to route
processing**
These three methods (`onAcceptAlertNotification`, `onAcceptDeliverSm`,
`onAcceptDataSm`) are called on jSMPP's I/O thread. After restoring the
interrupt flag, execution continues to `LOG.debug(...)` and then into route
processing — all of which may call blocking operations that will immediately
receive another `InterruptedException` because the flag is set.
The correct fix is to return early after restoring the flag:
```suggestion
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
return; // don't process the message if interrupted during
initialization
```
Same pattern applies to `onAcceptDeliverSm` (line 107) and `onAcceptDataSm`
(line 140).
--
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]
