oscerd opened a new issue, #743:
URL: https://github.com/apache/camel-karaf/issues/743
Split out of the review on #739, at @jbonofre's suggestion, so it does not
grow that PR. Two symptoms, one underlying cause: the delegate is invalidated
and stopped outside the monitor that `getDelegate()` takes, while callers
already hold a reference to it.
## 1. An invalidation can be swallowed
`removedService` writes `this.delegate = null` with no monitor held, so
`synchronized getDelegate()` gives mutual exclusion between readers only, never
against that writer:
```
A: getDelegate() takes `this`, delegate == null, enters createRegistry()
(loader L has already been loaded into the new registry)
B: removedService(L) -> stopService(this.delegate) // still null, no-op
-> this.delegate = null // no monitor held
A: delegate = <registry built with L in it>
```
The invalidation lands between A's read and A's write and is lost.
Converters belonging to an unregistered bundle keep serving indefinitely, and
no later event invalidates them.
Reproduced during the review by stalling at the end of `createRegistry()`:
after `removedService` returns, `getDelegate()` hands back the same instance
that was built with the removed loader in it, and `assertSame(built,
afterRemoval)` passes.
This predates #739 — the old unsynchronized `if (delegate == null)` had the
same lost write — but #739 makes the window **wider**, not narrower: the
monitor is now held across the whole rebuild, the core converter scan plus
every `loader.load()`, so there is much more time for a `removedService` to
land inside it.
An epoch counter would close it: read a generation before
`createRegistry()`, re-check it before the assign, discard and retry if
`removedService` bumped it meanwhile.
## 2. The outgoing delegate is stopped while callers still hold it
`getDelegate()` hands the delegate to a caller and returns; the caller then
uses it after the monitor is released. `removedService` can call
`ServiceHelper.stopService` on that same instance a moment later, so the caller
converts against a stopped registry.
Making `getDelegate()` fully `synchronized` in #739 narrows this window but
does not close it — the use is outside the monitor either way, so no locking
discipline on `getDelegate()` alone can fix it.
Closing it means not stopping the outgoing delegate eagerly: let the old
instance keep serving in-flight conversions and let it go when nothing
references it.
## Why together
Both are the same shape — mutation of an instance that callers already hold,
performed outside the monitor that publishes it — and a fix for one constrains
the fix for the other. Doing them in one change avoids designing the
invalidation twice.
Neither is a regression from #739; that PR is not blocked on this.
---
_Claude Code on behalf of Andrea Cosentino_
--
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]
