oscerd opened a new pull request, #25085:
URL: https://github.com/apache/camel/pull/25085
## The bug
`UtilizationStatistics.updateSpool()` calls `lock.lock()` again in its
`finally` block instead of `lock.unlock()`:
```java
void updateSpool(long size) {
lock.lock();
try {
spoolAverageSize.set(spoolSize.addAndGet(size) /
spoolCounter.incrementAndGet());
} finally {
lock.lock(); // should be unlock()
}
}
```
The sibling `updateMemory()` immediately above it is correct, so this reads
as a copy-paste slip.
Because the lock is a `ReentrantLock`, the calling thread does **not**
self-deadlock — it just leaves the hold count at 2 and never releases it. The
consequences are:
- the hold count grows without bound;
- **every other thread entering `updateSpool()` blocks forever**;
- **`CamelContext` shutdown deadlocks too** — `doStop()` calls
`statistics.reset()`, which acquires the same lock, so the thread stopping the
context also blocks.
`ReentrantLock.lock()` is uninterruptible, so neither `shutdownNow()` nor
interruption can recover a thread once it is stuck.
It is reachable whenever stream-caching statistics are enabled and a stream
is spooled to disk (`computeStatistics` → `updateSpool`, from `doCache`).
Memory-only caching takes the correct `updateMemory` path and is unaffected.
## The fix
One character: `lock.lock()` → `lock.unlock()`.
## The test
`StreamCachingStrategySpoolStatisticsTest` drives the spool accounting from
two threads. A single-threaded test cannot catch this, since a reentrant lock
lets its owner re-acquire it — only a *second* thread blocks.
Three deliberate choices keep a future regression from wedging CI rather
than failing it:
- **a standalone `DefaultStreamCachingStrategy`** instead of the
context-managed one. Using the context's instance would deadlock `CamelContext`
shutdown in `doStop()` → `statistics.reset()`, and the build would hang instead
of reporting a failure. I hit exactly that while developing this: an early
version wedged the surefire fork until its 50-minute timeout.
- **daemon worker threads**, so a permanently blocked thread cannot keep the
JVM alive.
- **a stub converter returning a cache that reports itself as spooled**,
exercising the spool accounting directly without depending on real disk
spooling (and therefore without depending on the context's spool configuration).
## Verification
Both directions were checked locally against freshly built artifacts:
- **with the fix** — passes.
- **with the one-line change reverted** — fails in seconds with `both
threads must finish; one blocked on the leaked spool statistics lock`.
---
_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]
