[jira] [Resolved] (CAMEL-24286) camel-support: BackgroundTask supplier exception hangs an unlimited-duration run() and leaks the task registration

["Andrea Cosentino (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/CAMEL-24286?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Andrea Cosentino resolved CAMEL-24286.
--------------------------------------
    Resolution: Fixed

Fixed on main via PR https://github.com/apache/camel/pull/25217. When a BackgroundTask supplier throws, the blocking run() latch is now released and the task is unregistered (previously a withUnlimitedDuration() task would await() forever and leak in the registry).

The same defect is present on camel-4.18.x and camel-4.14.x (their runTaskWrapper catch block rethrows without releasing the latch), but the surrounding code has diverged from main, so the fix does not cherry-pick cleanly and a backport needs to be adapted and tested against each branch. Flagging for a follow-up backport rather than porting a concurrency fix unadapted.

_Claude Code on behalf of Andrea Cosentino (@oscerd)._

> camel-support: BackgroundTask supplier exception hangs an unlimited-duration run() and leaks the task registration
> ------------------------------------------------------------------------------------------------------------------
>
>                 Key: CAMEL-24286
>                 URL: https://issues.apache.org/jira/browse/CAMEL-24286
>             Project: Camel
>          Issue Type: Bug
>          Components: camel-core
>            Reporter: Andrea Cosentino
>            Assignee: Andrea Cosentino
>            Priority: Major
>             Fix For: 4.22.0
>
>
> Follow-up defect from the ForegroundTask -> BackgroundTask reconnection-loop migration (CAMEL-24272 / CAMEL-24278, both merged).
> h3. Problem
> {{BackgroundTask.runTaskWrapper}} rethrows a supplier exception on the scheduler thread without releasing the completion latch or unregistering the task:
> {code:java}
> try {
>     if (doRun(supplier)) {
>         ...
>         latch.countDown();
>     }
> } catch (Exception e) {          // BackgroundTask.java ~131-134 on main
>     status = Status.Failed;
>     cause = e;
>     throw e;                     // rethrown onto the ScheduledExecutorService thread
> }
> {code}
> When the supplier throws a non-{{TaskRunFailureException}}:
> * the exception is rethrown onto the {{scheduleWithFixedDelay}} worker, which silently suppresses all further executions of that task (standard {{ScheduledExecutorService}} contract);
> * {{latch.countDown()}} is never reached, and {{registry.removeTask(this)}} is never called on this path;
> * 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*.
> {{ReentrantLock}} is not involved here, but note {{latch.await()}} in the unlimited case is likewise unrecoverable once the only countdown site is skipped.
> The interrupt path has a related smaller gap: in {{waitForTaskCompletion}}, {{registry.removeTask(this)}} sits inside the {{try}} (~line 205), while the {{finally}} only resets {{elapsed}}/{{running}}. An {{InterruptedException}} from {{latch.await()}} therefore also leaves the task registered.
> h3. Reachability on main (post CAMEL-24272)
> Both of these now route an *unlimited-duration* reconnection through {{BackgroundTask.run()}}:
> * *camel-ftp SFTP* — {{SftpOperations.java:142}} uses {{withUnlimitedDuration()}}; {{tryConnect}} catches only {{JSchException}} ({{:209}}, {{:221}}). A non-JSch {{RuntimeException}} on the connect path (e.g. an invalid {{filenameEncoding}} reaching {{Charset.forName}}) escapes into the throw path above -> hang + registry leak.
> * *camel-mongodb-gridfs* — {{GridFsConsumer.java:122}} {{withUnlimitedDuration()}}, {{:131}} {{task.run(...)}}. In {{processCollection}} the inner {{catch (Exception) { // ignore }}} ({{:184}}) wraps *only* {{getProcessor().process(exchange)}}; the cursor acquisition {{getGridFSFileMongoCursor(...)}} ({{:143}}) and the pre-process {{findOneAndUpdate(...)}} ({{:155}}) are *outside* any catch. A transient {{MongoException}} there escapes -> same hang + leak.
> Bounded tasks ({{withMaxDuration}}, e.g. Infinispan) do not hang because {{latch.await(maxDuration)}} times out -- but they still fail slow and lose the real cause. Suppliers that catch-all and return {{false}} (e.g. FTP {{tryConnect}}) are unaffected.
> h3. Suggested fix
> Treat a thrown supplier exception as a terminal failure that still releases the caller:
> * in the {{catch}} block, set {{completed.set(false)}}, unregister when {{!registeredByRun}}, and call {{latch.countDown()}} (do not rely on rethrowing onto the scheduler thread, which serves no purpose there);
> * move {{registry.removeTask(this)}} in {{waitForTaskCompletion}} into the {{finally}} so the interrupt path also unregisters.
> A regression test can build an unlimited-duration {{BackgroundTask}} whose supplier throws on the first run and assert (with a bounded await, daemon thread) that {{run()}} returns and the registry ends empty -- mirroring the shape used in CAMEL-24244's {{StreamCachingStrategySpoolStatisticsTest}}.
> h3. Notes
> Found while reviewing the task-manager PR cluster (#25164 merged, #25169 closed). Not attributable to a single PR -- it is the residual foreground->background porting gap now reachable on main. Raising for the owners of CAMEL-2427x to assess, since some of the rethrow behaviour may be intentional for programming errors; the latch-release/unregister omission is the part that looks unintended.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)