joerghoh opened a new pull request, #70: URL: https://github.com/apache/sling-org-apache-sling-servlets-resolver/pull/70
### Context `BundledScriptTracker` maintains the `DispatcherServlet` registrations that route requests to bundled scripts, keyed by resource type. It rebuilds this map in `refreshDispatcher()`, which is invoked from the OSGi `BundleTracker` callbacks `addingBundle`/`removedBundle` — and those can be dispatched on **different threads**. This matters during startup and coordinated deployments, when several script-carrying bundles activate or refresh at (nearly) the same time. ### The problem `refreshDispatcher()` did a non-atomic *read → mutate-in-place → swap* on the shared `dispatchers` map: it read the published map, called `remove(rt)` on that same map to decide which dispatchers to reuse, and only then swapped in a freshly built map. Two concurrent invocations therefore read and drained the *same* published map against each other, so a resource type one thread had already claimed looked absent to the other, which re-registered a duplicate `DispatcherServlet` (and the trailing `set(...)` lost one result). The user-visible symptom is intermittent and timing-dependent: after concurrent bundle install/uninstall, resource types end up with duplicate or missing dispatcher registrations, so bundled scripts stop resolving (falling through to the default servlet) or a stale script keeps being served. ### The fix `refreshDispatcher()` is now `synchronized`, so the read/compute/swap runs as one atomic unit — this reconcile-to-desired-state routine is inherently serial and runs only on rare bundle-lifecycle events, so there is no meaningful contention. Additionally it now builds from a copy of the published map (swapped in atomically at the end) rather than mutating it in place, and guards against a null map after deactivation. ### Unit test `BundledScriptTrackerRaceConditionTest` exercises the defect by firing many concurrent `refreshDispatcher()` calls (barrier-aligned, over 200 rounds) with the *same* set of registrations, and asserting idempotency: each resource type's dispatcher must be registered exactly once and reused thereafter, with nothing spuriously unregistered. On the old code this fails immediately (dozens of duplicate registrations, or an exception from the concurrently mutated `HashMap`); with the fix it passes. The test drives the real component lifecycle (`activate()` + a real `ServletMounter`) and observes behavior through register/unregister counters rather than reflection; the OSGi collaborators are stubbed with `java.lang.reflect.Proxy` because `ServiceReference`/`Bundle` extend `Comparable`, which the mocking engine cannot instrument on recent JDKs (the production code stubs `ServiceReference` the same way). `refreshDispatcher` and the `mounter` reference were made package-private so the test c an drive them directly. -- 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]
