Ankur Singla created FELIX-6850:
-----------------------------------
Summary: ConfigAdmin: Race condition in ConfigurationAdminStarter
activate/deactivate causes NPE and inconsistent Configuration Admin registration
Key: FELIX-6850
URL: https://issues.apache.org/jira/browse/FELIX-6850
Project: Felix
Issue Type: Bug
Components: Configuration Admin
Affects Versions: configadmin-1.9.26
Reporter: Ankur Singla
Fix For: configadmin-1.9.28
h2. Summary
{{ConfigurationAdminStarter.activate()}} and {{deactivate()}} mutate the shared
{{configurationManager}} field without mutual exclusion. When they run
concurrently on different threads, {{activate()}} can throw a
{{{}NullPointerException{}}}, and — even without the NPE — the two methods can
interleave so that a {{ConfigurationAdmin}} service is left orphaned (started
but
no longer tracked) or registered twice. Because so many components wait on
Configuration Admin, this can wedge a large part of the framework.
h2. Affects Version/s
This is a latent defect, not a recent regression. The affected code was
introduced with the required-configuration-plugin gating in FELIX-6059 and first
shipped in {*}1.9.18{*}. It is unchanged in every release since, up to and
including
*1.9.26* and current {{master}} (1.9.27-SNAPSHOT).
h2. Symptom
Observed as a framework error during bundle refresh / startup:
{code:java}
*ERROR* ... FrameworkEvent ERROR (java.lang.NullPointerException:
Cannot invoke
"org.apache.felix.cm.impl.ConfigurationManager.updateRegisteredConfigurationPlugins(String)"
because "this.configurationManager" is null)
{code}
h2. Root cause
{{ConfigurationAdminStarter}} exposes several lifecycle methods that all mutate
{{configurationManager}} / {{persistenceManager}} but share no common lock:
* Persistence-manager path: {{PersistenceManagerTracker}} calls
{{setPersistenceManager()}} / {{unsetPersistenceManager()}} while holding its
own {{holders}} monitor.
* Configuration-plugin path: {{RequiredConfigurationPluginTracker}} calls
{{updatePluginsSet()}} while holding no shared lock.
* Bundle stop: {{Activator.stop()}} -> {{DependencyTracker.stop()}} ->
{{{}deactivate(){}}}.
These run on independent threads, so {{activate()}} and {{deactivate()}} can
overlap. In {{activate()}} the {{configurationManager}} field is assigned and
then re-read on later lines:
{code:java}
configurationManager = new ConfigurationManager(pm, bundleContext);
this.startCoordinatorTracker();
final ServiceReference<ConfigurationAdmin> ref = configurationManager.start();
configurationManager.updateRegisteredConfigurationPlugins(this.registeredConfigurationPlugins);
// NPE
{code}
If {{deactivate()}} runs on another thread between the assignment and the
re-reads (it sets the field to {{{}null{}}}), the re-read throws the NPE
reported
above. Even when the NPE is avoided, the lack of mutual exclusion means an
{{activate()}} that completes after a concurrent {{deactivate()}} leaves a
started-but-untracked ConfigurationAdmin (resource/thread leak), or a second
ConfigurationAdmin registered — an inconsistent state that never recovers.
A secondary, related issue: the optional Coordinator {{ServiceTracker}}
callbacks in {{startCoordinatorTracker()}} dereference {{configurationManager}}
directly without a null check, which can NPE on coordinator service churn during
shutdown.
h2. When it triggers
Most easily reproduced when {{felix.cm.config.plugins}} requires one or more
ConfigurationPlugins (so activation is gated on plugin/PM availability) and
those services — or the Config Admin bundle itself — are refreshed/flap, e.g.
during a large bundle-refresh cycle or product upgrade. The concurrent
plugin/PM/stop callbacks drive {{activate()}} and {{deactivate()}} in parallel.
h2. Proposed fix
In {{{}ConfigurationAdminStarter{}}}:
# In {{{}activate(){}}}, capture the newly created {{ConfigurationManager}} in
a
local variable and operate on that local (instead of re-reading the field), so
an interleaving {{deactivate()}} cannot cause an NPE.
# Serialize the lifecycle-transition methods ({{{}activate{}}},
{{{}deactivate{}}},
{{{}checkStart{}}}, {{{}updatePluginsSet{}}}, {{{}setPersistenceManager{}}},
{{{}unsetPersistenceManager{}}}, {{{}updateRegisteredConfigurationPlugins{}}})
so
{{{}activate{}}}/{{{}deactivate{}}} cannot interleave and leave an orphaned or
duplicate registration. This is consistent with the existing
PersistenceManagerTracker locking and is deadlock-safe: no path acquires the
starter monitor and then {{holders}} (only the reverse), and the update/event
threads and CA-registration listeners do not call back into the starter.
# Guard the Coordinator {{ServiceTracker}} callbacks against a null
{{{}configurationManager{}}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
