[
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)
