From d700253501f36b049c2cef79e95fca7abd0d85ab Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Mon, 20 Sep 2021 17:39:21 +0200 Subject: [PATCH] Fix OSGiDOMSchemaService lifecycle Original refactor specified rather invalid lifecycle for the OSGiModuleInfoSnapshot dependency -- it is missing the dynamic binding which was there in one version of the patch set. Unfortunately the invalid instruction (FieldOption for a method) is not caught and it is causing a rather weird state, where we end up with a deactivated service still having listeners firing. Fix injection lifecycle by having a property DYNAMIC and GREEDY lifecycle -- which does exactly the right thing, except we need to track a bit more state to make logging more sane. JIRA: MDSAL-689 Change-Id: Ia027c45daab528821bee9ae93248d9d7e9ada85d Signed-off-by: Robert Varga (cherry picked from commit 7484e78692a8a26d02210917b7aec7c28fb317bb) --- .../osgi/impl/OSGiDOMSchemaService.java | 30 ++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/dom/mdsal-dom-schema-osgi/src/main/java/org/opendaylight/mdsal/dom/schema/osgi/impl/OSGiDOMSchemaService.java b/dom/mdsal-dom-schema-osgi/src/main/java/org/opendaylight/mdsal/dom/schema/osgi/impl/OSGiDOMSchemaService.java index 567f10a862..267c9de96a 100644 --- a/dom/mdsal-dom-schema-osgi/src/main/java/org/opendaylight/mdsal/dom/schema/osgi/impl/OSGiDOMSchemaService.java +++ b/dom/mdsal-dom-schema-osgi/src/main/java/org/opendaylight/mdsal/dom/schema/osgi/impl/OSGiDOMSchemaService.java @@ -12,6 +12,7 @@ import static java.util.Objects.requireNonNull; import com.google.common.util.concurrent.ListenableFuture; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicReference; import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.mdsal.binding.runtime.api.ModuleInfoSnapshot; import org.opendaylight.mdsal.dom.api.DOMSchemaService; @@ -27,7 +28,6 @@ import org.osgi.service.component.ComponentInstance; import org.osgi.service.component.annotations.Activate; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Deactivate; -import org.osgi.service.component.annotations.FieldOption; import org.osgi.service.component.annotations.Reference; import org.osgi.service.component.annotations.ReferenceCardinality; import org.osgi.service.component.annotations.ReferencePolicy; @@ -46,12 +46,13 @@ public final class OSGiDOMSchemaService extends AbstractDOMSchemaService.WithYan ComponentFactory listenerFactory = null; private final List listeners = new CopyOnWriteArrayList<>(); + private final AtomicReference currentSnapshot = new AtomicReference<>(); - private volatile ModuleInfoSnapshot currentSnapshot; + private boolean deactivated; @Override - public EffectiveModelContext getGlobalContext() { - return currentSnapshot.getEffectiveModelContext(); + public @NonNull EffectiveModelContext getGlobalContext() { + return currentSnapshot.get().getEffectiveModelContext(); } @Override @@ -62,18 +63,27 @@ public final class OSGiDOMSchemaService extends AbstractDOMSchemaService.WithYan @Override public ListenableFuture getSource(final SourceIdentifier sourceIdentifier) { - return currentSnapshot.getSource(sourceIdentifier); + return currentSnapshot.get().getSource(sourceIdentifier); } - @Reference(fieldOption = FieldOption.REPLACE) + @Reference(policy = ReferencePolicy.DYNAMIC, policyOption = ReferencePolicyOption.GREEDY) void bindSnapshot(final OSGiModuleInfoSnapshot newContext) { - LOG.trace("Updating context to generation {}", newContext.getGeneration()); + LOG.info("Updating context to generation {}", newContext.getGeneration()); final ModuleInfoSnapshot snapshot = newContext.getService(); final EffectiveModelContext ctx = snapshot.getEffectiveModelContext(); - currentSnapshot = snapshot; + final ModuleInfoSnapshot previous = currentSnapshot.getAndSet(snapshot); + LOG.debug("Snapshot updated from {} to {}", previous, snapshot); + listeners.forEach(listener -> notifyListener(ctx, listener)); } + void unbindSnapshot(final OSGiModuleInfoSnapshot oldContext) { + final ModuleInfoSnapshot snapshot = oldContext.getService(); + if (currentSnapshot.compareAndSet(snapshot, null) && !deactivated) { + LOG.info("Lost final generation {}", oldContext.getGeneration()); + } + } + @Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC, policyOption = ReferencePolicyOption.GREEDY) void addListener(final EffectiveModelContextListener listener) { @@ -94,9 +104,9 @@ public final class OSGiDOMSchemaService extends AbstractDOMSchemaService.WithYan } @Deactivate - @SuppressWarnings("static-method") void deactivate() { LOG.info("DOM Schema services deactivated"); + deactivated = true; } private @NonNull ListenerRegistration registerListener( @@ -116,7 +126,7 @@ public final class OSGiDOMSchemaService extends AbstractDOMSchemaService.WithYan } @SuppressWarnings("checkstyle:illegalCatch") - private static void notifyListener(final EffectiveModelContext context, + private static void notifyListener(final @NonNull EffectiveModelContext context, final EffectiveModelContextListener listener) { try { listener.onModelContextUpdated(context); -- 2.36.6