oscerd opened a new pull request, #25217:
URL: https://github.com/apache/camel/pull/25217
## The bug
`BackgroundTask.runTaskWrapper` rethrows a supplier exception on the
scheduler thread without counting down the completion latch or unregistering
the task:
```java
try {
if (doRun(supplier)) {
...
latch.countDown();
}
} catch (Exception e) { // no countDown, no removeTask
status = Status.Failed;
cause = e;
throw e;
}
```
`doRun` only catches `TaskRunFailureException`, so any *other* exception
reaches this path. `scheduleWithFixedDelay` then suppresses all further
executions, and for a task built with `withUnlimitedDuration()`
`waitForTaskCompletion` calls `latch.await()` with **no timeout** — so the
calling thread **blocks forever** and the task **leaks in the
`TaskManagerRegistry`**.
The interrupt path has a smaller sibling of the same gap: in
`waitForTaskCompletion`, `removeTask` sits inside the `try`, so an
`InterruptedException` from `latch.await()` also leaves the task registered.
## Reachability on `main`
CAMEL-24272 now routes two *unlimited-duration* reconnection loops through
`BackgroundTask.run()`:
- **camel-ftp SFTP** — `SftpOperations.java` uses `withUnlimitedDuration()`;
`tryConnect` catches only `JSchException`. A non-JSch `RuntimeException` on the
connect path escapes into the throw path.
- **camel-mongodb-gridfs** — `GridFsConsumer` uses `withUnlimitedDuration()`
+ `run()`. In `processCollection`, the inner `catch (Exception) { // ignore }`
wraps only `getProcessor().process(...)`; the cursor acquisition and the
pre-process `findOneAndUpdate` are **outside** any catch, so a transient
`MongoException` escapes.
Bounded tasks (`withMaxDuration`, e.g. Infinispan) don't hang —
`latch.await(maxDuration)` times out — but they still fail slow and lose the
cause.
## The fix
In the `catch`, mark the task not-completed, unregister it, and count down
the latch **before** rethrowing, so the blocking `run()` caller unblocks and
observes the failure. The rethrow is kept so the `schedule()` path (no latch
waiter) still surfaces the error to the executor.
Also move `removeTask` + `task.cancel` into the `finally` of
`waitForTaskCompletion`, so an interrupted `await()` no longer leaves the task
registered.
## Test
`BackgroundTaskRegistryTest.testRunReturnsAndUnregistersWhenSupplierThrows`
builds an unlimited-duration task whose supplier throws, runs it on a
**daemon** thread (so a regression can't wedge the JVM), and asserts `run()`
returns, reports not-completed, and the registry ends empty.
Verified both directions locally:
- **without the fix** — the new test fails (`run()` hangs on the 10s join);
the three existing tests pass.
- **with the fix** — all four pass.
## Notes
Found while reviewing the task-manager PR cluster (CAMEL-24271/24272/24278).
It is the residual foreground→background porting gap, not attributable to a
single PR. The rethrow is deliberately preserved in case surfacing the error to
the executor on the `schedule()` path is intended — only the
latch-release/unregister omission is fixed.
---
_Claude Code on behalf of @oscerd_
--
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]
