[
https://issues.apache.org/jira/browse/CAMEL-24244?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Claus Ibsen resolved CAMEL-24244.
---------------------------------
Resolution: Fixed
> camel-base-engine: DefaultStreamCachingStrategy.updateSpool calls lock()
> instead of unlock() in finally, blocking other threads forever
> ---------------------------------------------------------------------------------------------------------------------------------------
>
> Key: CAMEL-24244
> URL: https://issues.apache.org/jira/browse/CAMEL-24244
> Project: Camel
> Issue Type: Bug
> Components: camel-core
> Reporter: Andrea Cosentino
> Assignee: Andrea Cosentino
> Priority: Major
> Fix For: 4.14.9, 4.18.4, 4.20.0
>
>
> {{DefaultStreamCachingStrategy.UtilizationStatistics.updateSpool(long)}}
> calls {{lock.lock()}} in its {{finally}} block instead of {{lock.unlock()}},
> so the lock is never released.
> h3. Code
> {{core/camel-base-engine/src/main/java/org/apache/camel/impl/engine/DefaultStreamCachingStrategy.java}}:
> {noformat}
> private final Lock lock = new ReentrantLock(); // line 563
> void updateMemory(long size) { // line 572 -- correct
> lock.lock();
> try {
> memoryAverageSize.set(memorySize.addAndGet(size) /
> memoryCounter.incrementAndGet());
> } finally {
> lock.unlock();
> }
> }
> void updateSpool(long size) { // line 581 -- BUG
> lock.lock();
> try {
> spoolAverageSize.set(spoolSize.addAndGet(size) /
> spoolCounter.incrementAndGet());
> } finally {
> lock.lock(); // line 586 -- should be
> unlock()
> }
> }
> {noformat}
> The sibling {{updateMemory}} immediately above is correct, which makes this
> look like a copy-paste slip.
> h3. Impact
> {{lock}} is a {{ReentrantLock}}, so the calling thread does not deadlock
> against itself -- it simply leaves the hold count at 2 and never releases it.
> The consequences:
> * The hold count grows without bound across invocations on the same thread.
> * **Any other thread entering {{updateSpool()}} blocks forever.** With more
> than one routing thread performing stream caching, this is a permanent hang
> of those threads.
> h3. Reachability
> {{updateSpool}} is called from {{computeStatistics}} (line 356), which runs
> from {{doCache}} (line 316) under:
> {noformat}
> if (statistics.isStatisticsEnabled()) {
> computeStatistics(cache);
> }
> {noformat}
> So the bug is reachable whenever stream-caching statistics are enabled *and*
> a stream is spooled to disk (rather than kept in memory). Memory-only caching
> takes the {{updateMemory}} path and is unaffected.
> h3. Suggested fix
> Change line 586 to {{lock.unlock();}}.
> A regression test would need two threads calling {{updateSpool}} concurrently
> with a timeout, asserting both complete -- the current single-threaded tests
> cannot catch it because reentrancy hides the bug from the first thread.
> h3. Notes
> Found while reviewing PR #24983 / #24985 (CAMEL-24227), which touches this
> file for an unrelated {{volatile}} change. Not introduced by that PR -- it is
> pre-existing on {{main}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
