This is an automated email from the ASF dual-hosted git repository.
jbonofre pushed a commit to branch karaf-4.4.x
in repository https://gitbox.apache.org/repos/asf/karaf.git
The following commit(s) were added to refs/heads/karaf-4.4.x by this push:
new 0debcb0b53 Do not restart an activator when nothing changed since it
started (#2974)
0debcb0b53 is described below
commit 0debcb0b535cc779313bc61fac426f5d117d6dd8
Author: JB Onofré <[email protected]>
AuthorDate: Tue Oct 6 06:18:55 2026 +0200
Do not restart an activator when nothing changed since it started (#2974)
BaseActivator schedules a run (doStop + doStart) each time a tracked
service or the configuration changes. When a tracked service showed up
while a run was in progress, doStart could already see it and start the
activator completely. The run scheduled for that same change then stopped
and started everything again.
For the features service, this happens when the mvn URL handler is
registered while the activator processes its first run: the boot features
are installed, BootFinished is published, then the FeaturesService and the
region hooks are unregistered and registered again. Anything done at that
moment can fail, and this is when the integration tests install their
probe:
- the resolver hook is registered before the probe is assigned to a
region, so its resolution is prevented
- the FeaturesService the test got has been stopped, so installing a
feature is rejected
doStart now works on the tracked services and the configuration read at
the beginning of the run, and a run is skipped when they are the ones the
activator has already been started with.
---
.../apache/karaf/util/tracker/BaseActivator.java | 85 ++++++++-
.../karaf/util/tracker/BaseActivatorTest.java | 192 +++++++++++++++++++++
2 files changed, 269 insertions(+), 8 deletions(-)
diff --git
a/util/src/main/java/org/apache/karaf/util/tracker/BaseActivator.java
b/util/src/main/java/org/apache/karaf/util/tracker/BaseActivator.java
index de39cb0844..484e78a3cb 100644
--- a/util/src/main/java/org/apache/karaf/util/tracker/BaseActivator.java
+++ b/util/src/main/java/org/apache/karaf/util/tracker/BaseActivator.java
@@ -25,6 +25,7 @@ import java.util.HashMap;
import java.util.Hashtable;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Properties;
import java.util.Queue;
import java.util.concurrent.ConcurrentLinkedQueue;
@@ -58,7 +59,12 @@ public class BaseActivator implements BundleActivator,
Runnable, ThreadFactory {
private final Queue<ServiceRegistration> registrations = new
ConcurrentLinkedQueue<>();
private Map<String, SingleServiceTracker> trackers = new HashMap<>();
private ServiceRegistration managedServiceRegistration;
- private Dictionary<String, ?> configuration;
+ private volatile Dictionary<String, ?> configuration;
+
+ /** What the activator has been started with, {@code null} if it is not
started. */
+ private volatile State started;
+ /** What {@link #doStart()} is currently working with, {@code null}
outside of {@link #doStart()}. */
+ private volatile State starting;
private static final AtomicInteger poolNumber = new AtomicInteger(1);
private final ThreadGroup group;
@@ -85,13 +91,13 @@ public class BaseActivator implements BundleActivator,
Runnable, ThreadFactory {
scheduled.set(true);
doOpen();
scheduled.set(false);
- if (managedServiceRegistration == null
- && trackers.values().stream()
- .allMatch(t -> t.getService() != null)) {
+ State state = new State();
+ if (managedServiceRegistration == null && state.hasAllServices()) {
try {
- doStart();
+ doStart(state);
} catch (Throwable e) {
logger.warn("Error starting activator", e);
+ started = null;
doStop();
}
} else {
@@ -158,6 +164,10 @@ public class BaseActivator implements BundleActivator,
Runnable, ThreadFactory {
if (properties != null) {
this.configuration = properties;
+ State state = starting;
+ if (state != null) {
+ state.configuration = properties;
+ }
return true;
}
}
@@ -307,15 +317,72 @@ public class BaseActivator implements BundleActivator,
Runnable, ThreadFactory {
@Override
public void run() {
scheduled.set(false);
+ // The state is read after the flag has been reset: a change happening
from now on
+ // schedules another run, and a change that happened before is part of
this state.
+ State state = new State();
+ if (state.sameAs(started)) {
+ // The change this run has been scheduled for was already visible
to the previous
+ // start, so there is nothing to do. Restarting would needlessly
unregister, and
+ // register again, everything that has just been started.
+ return;
+ }
doStop();
try {
- doStart();
+ doStart(state);
} catch (Exception e) {
logger.warn("Error starting activator", e);
+ started = null;
doStop();
}
}
+ private void doStart(State state) throws Exception {
+ started = state;
+ starting = state;
+ try {
+ doStart();
+ } finally {
+ starting = null;
+ }
+ }
+
+ /**
+ * The configuration and the tracked services at a given point in time.
+ * <p>
+ * {@link #doStart()} works on such a state rather than on the trackers,
so that
+ * what the activator has been started with is known for sure.
+ */
+ private final class State {
+
+ private volatile Dictionary<String, ?> configuration =
BaseActivator.this.configuration;
+ private final Map<String, ServiceReference> references = new
HashMap<>();
+ private final Map<String, Object> services = new HashMap<>();
+
+ State() {
+ for (Map.Entry<String, SingleServiceTracker> entry :
trackers.entrySet()) {
+ references.put(entry.getKey(),
entry.getValue().getServiceReference());
+ services.put(entry.getKey(), entry.getValue().getService());
+ }
+ }
+
+ boolean hasAllServices() {
+ return services.values().stream().allMatch(Objects::nonNull);
+ }
+
+ boolean sameAs(State other) {
+ if (other == null || configuration != other.configuration) {
+ return false;
+ }
+ for (String name : services.keySet()) {
+ if (services.get(name) != other.services.get(name)
+ || !Objects.equals(references.get(name),
other.references.get(name))) {
+ return false;
+ }
+ }
+ return true;
+ }
+ }
+
/**
* Called in {@link #doOpen()}.
*
@@ -368,7 +435,8 @@ public class BaseActivator implements BundleActivator,
Runnable, ThreadFactory {
if (tracker == null) {
throw new IllegalStateException("Service not tracked for class " +
clazz);
}
- return clazz.cast(tracker.getService());
+ State state = starting;
+ return clazz.cast(state != null ? state.services.get(clazz.getName())
: tracker.getService());
}
protected <T> ServiceReference<T> getTrackedServiceRef(Class<T> clazz) {
@@ -376,7 +444,8 @@ public class BaseActivator implements BundleActivator,
Runnable, ThreadFactory {
if (tracker == null) {
throw new IllegalStateException("Service not tracked for class " +
clazz);
}
- return tracker.getServiceReference();
+ State state = starting;
+ return state != null ? state.references.get(clazz.getName()) :
tracker.getServiceReference();
}
/**
diff --git
a/util/src/test/java/org/apache/karaf/util/tracker/BaseActivatorTest.java
b/util/src/test/java/org/apache/karaf/util/tracker/BaseActivatorTest.java
new file mode 100644
index 0000000000..9574508fe3
--- /dev/null
+++ b/util/src/test/java/org/apache/karaf/util/tracker/BaseActivatorTest.java
@@ -0,0 +1,192 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.karaf.util.tracker;
+
+import static org.easymock.EasyMock.anyObject;
+import static org.easymock.EasyMock.anyString;
+import static org.easymock.EasyMock.capture;
+import static org.easymock.EasyMock.eq;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.expectLastCall;
+import static org.easymock.EasyMock.getCurrentArguments;
+import static org.easymock.EasyMock.newCapture;
+import static org.easymock.EasyMock.niceMock;
+import static org.easymock.EasyMock.replay;
+import static org.junit.Assert.assertEquals;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Hashtable;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.TimeUnit;
+
+import org.easymock.Capture;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.ServiceEvent;
+import org.osgi.framework.ServiceListener;
+import org.osgi.framework.ServiceReference;
+
+public class BaseActivatorTest {
+
+ private final Map<ServiceReference<?>, Runnable> services = new
ConcurrentHashMap<>();
+ private volatile ServiceReference<?> registered;
+
+ private BundleContext bundleContext;
+ private Capture<ServiceListener> listener;
+ private TestActivator activator;
+
+ @Before
+ public void setUp() throws Exception {
+ Bundle bundle = niceMock(Bundle.class);
+ bundleContext = niceMock(BundleContext.class);
+ listener = newCapture();
+ expect(bundleContext.getBundle()).andStubReturn(bundle);
+ bundleContext.addServiceListener(capture(listener), anyString());
+ expectLastCall().asStub();
+
expect(bundleContext.getServiceReferences(eq(Runnable.class.getName()),
anyObject()))
+ .andStubAnswer(() -> registered != null ? new
ServiceReference<?>[] { registered } : null);
+ expect(bundleContext.getService(anyObject()))
+ .andStubAnswer(() -> services.get(getCurrentArguments()[0]));
+ replay(bundle, bundleContext);
+
+ activator = new TestActivator();
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ activator.stop(bundleContext);
+ }
+
+ @Test
+ public void startsOnceWhenServiceShowsUpWhileStarting() throws Exception {
+ // the tracked service is registered while the activator is processing
a previous change
+ ServiceReference<?> reference = reference();
+ activator.onStop = () -> register(reference);
+
+ activator.start(bundleContext);
+ activator.await();
+
+ assertEquals(Collections.singletonList(reference), activator.starts);
+ }
+
+ @Test
+ public void startsWhenServiceShowsUpLater() throws Exception {
+ ServiceReference<?> reference = reference();
+
+ activator.start(bundleContext);
+ activator.await();
+ assertEquals(Collections.emptyList(), activator.starts);
+
+ register(reference);
+ activator.await();
+
+ assertEquals(Collections.singletonList(reference), activator.starts);
+ }
+
+ @Test
+ public void restartsWhenServiceIsReplaced() throws Exception {
+ ServiceReference<?> first = reference();
+ ServiceReference<?> second = reference();
+ registered = first;
+
+ activator.start(bundleContext);
+ activator.await();
+ assertEquals(Collections.singletonList(first), activator.starts);
+
+ registered = second;
+ listener.getValue().serviceChanged(new
ServiceEvent(ServiceEvent.UNREGISTERING, first));
+ activator.await();
+
+ assertEquals(Arrays.asList(first, second), activator.starts);
+ }
+
+ @Test
+ public void restartsWhenConfigurationIsUpdated() throws Exception {
+ ServiceReference<?> reference = reference();
+ registered = reference;
+
+ activator.start(bundleContext);
+ activator.await();
+ assertEquals(Collections.singletonList(reference), activator.starts);
+
+ activator.updated(new Hashtable<>());
+ activator.await();
+
+ assertEquals(Arrays.asList(reference, reference), activator.starts);
+ }
+
+ private ServiceReference<?> reference() {
+ ServiceReference<?> reference = niceMock(ServiceReference.class);
+ replay(reference);
+ services.put(reference, () -> { });
+ return reference;
+ }
+
+ private void register(ServiceReference<?> reference) {
+ registered = reference;
+ listener.getValue().serviceChanged(new
ServiceEvent(ServiceEvent.REGISTERED, reference));
+ }
+
+ /**
+ * Tracks a {@link Runnable} service and records the references it has
been started with.
+ */
+ private static class TestActivator extends BaseActivator {
+
+ final List<ServiceReference<?>> starts =
Collections.synchronizedList(new ArrayList<>());
+ volatile Runnable onStop;
+
+ @Override
+ protected void doOpen() throws Exception {
+ trackService(Runnable.class);
+ }
+
+ @Override
+ protected void doStart() throws Exception {
+ if (getTrackedService(Runnable.class) != null) {
+ starts.add(getTrackedServiceRef(Runnable.class));
+ }
+ }
+
+ @Override
+ protected void doStop() {
+ Runnable action = onStop;
+ onStop = null;
+ if (action != null) {
+ action.run();
+ }
+ super.doStop();
+ }
+
+ /**
+ * Waits for the changes notified so far to be processed.
+ */
+ void await() throws Exception {
+ // a change notified while another one is processed is queued
behind the first marker
+ for (int i = 0; i < 2; i++) {
+ executor.submit(() -> { }).get(10, TimeUnit.SECONDS);
+ }
+ }
+ }
+
+}
