This is an automated email from the ASF dual-hosted git repository.
jbonofre pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/karaf.git
The following commit(s) were added to refs/heads/main by this push:
new 1f081a7719 Do not restart an activator when nothing changed since it
started (#2973)
1f081a7719 is described below
commit 1f081a77192647286519b47eed3f47bef80e402e
Author: JB Onofré <[email protected]>
AuthorDate: Sun Oct 4 17:37:59 2026 +0200
Do not restart an activator when nothing changed since it started (#2973)
---
.../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);
+ }
+ }
+ }
+
+}
