This is an automated email from the ASF dual-hosted git repository. royteeuwen pushed a commit to branch feature/SLING-13274-1.x in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git
commit 5da8528f4bf00895bed33aff4425932a47b2c51f Author: Roy Teeuwen <[email protected]> AuthorDate: Tue Jul 28 09:23:09 2026 +0200 SLING-13274: MapEntries: NPE in onChange when a resource change is delivered during construction or disposal (backport to 1.x) Cherry-picked from master (3b1608f) since this fix was never backported to the 1.x maintenance branch. --- .../resourceresolver/impl/mapping/MapEntries.java | 22 ++++++--- .../impl/mapping/MapEntriesTest.java | 54 ++++++++++++++++++++++ 2 files changed, 69 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java index 55b06869..1784481b 100644 --- a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java +++ b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java @@ -142,10 +142,15 @@ public class MapEntries implements MapEntriesHandler, ResourceChangeListener, Ex this::drainAliasQueue); this.ah.initializeAliases(); - this.registration = registerResourceChangeListener(bundleContext); - + // create the vanity path handler before registering the resource change listener: the + // listener may be called as soon as it is registered, and onChange requires both handlers + // to be present (events arriving before initializeVanityPaths completes are queued and + // drained by the initializer) this.vph = new VanityPathHandler(this.factory, this.resolveMapsMap, this.initializing, this::drainVanityPathQueue); + + this.registration = registerResourceChangeListener(bundleContext); + this.vph.initializeVanityPaths(); if (metrics.isPresent()) { @@ -293,16 +298,19 @@ public class MapEntries implements MapEntriesHandler, ResourceChangeListener, Ex */ public void dispose() { - if (this.ah != null) { - ah.dispose(); - ah = null; - } - + // unregister the resource change listener before disposing the alias handler: onChange + // requires the handlers to be present, so they must not be torn down while the listener + // can still be called if (this.registration != null) { this.registration.unregister(); this.registration = null; } + if (this.ah != null) { + ah.dispose(); + ah = null; + } + /* * Cooperation with doInit: The same lock as used by doInit is acquired * thus preventing doInit from running and waiting for a concurrent diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java index 237c5109..387a3657 100644 --- a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java +++ b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java @@ -37,6 +37,8 @@ import java.util.concurrent.atomic.AtomicInteger; import org.apache.commons.lang3.StringUtils; import org.apache.sling.api.resource.Resource; import org.apache.sling.api.resource.ResourceResolver; +import org.apache.sling.api.resource.observation.ResourceChange; +import org.apache.sling.api.resource.observation.ResourceChangeListener; import org.apache.sling.api.resource.path.Path; import org.apache.sling.resourceresolver.impl.ResourceResolverMetrics; import org.junit.After; @@ -48,6 +50,7 @@ import org.mockito.MockitoAnnotations; import org.mockito.stubbing.Answer; import org.osgi.framework.Bundle; import org.osgi.framework.BundleContext; +import org.osgi.framework.ServiceRegistration; import org.osgi.service.event.EventAdmin; import static org.junit.Assert.assertEquals; @@ -223,6 +226,57 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest { mapEntries.ah.initializeAliases(); } + // tests SLING-13236: a resource change delivered while the MapEntries constructor is still + // running (the listener may be called as soon as it is registered) must not cause an NPE on + // the not-yet-assigned VanityPathHandler + @Test + public void testChangeDeliveredDuringListenerRegistration() throws Exception { + final AtomicBoolean changeDelivered = new AtomicBoolean(false); + when(bundleContext.registerService(eq(ResourceChangeListener.class), any(ResourceChangeListener.class), any())) + .thenAnswer((Answer<ServiceRegistration<ResourceChangeListener>>) invocation -> { + final ResourceChangeListener listener = invocation.getArgument(1); + listener.onChange(List.of(new ResourceChange(ResourceChange.ChangeType.ADDED, "/resource", false))); + changeDelivered.set(true); + return null; + }); + + final MapEntries entries = new MapEntries( + resourceResolverFactory, bundleContext, eventAdmin, stringInterpolationProvider, Optional.empty()); + try { + assertTrue("change should have been delivered during listener registration", changeDelivered.get()); + } finally { + entries.dispose(); + } + } + + // tests SLING-13236: a resource change delivered while dispose() is unregistering the + // listener must not cause an NPE on an already torn down AliasHandler + @Test + @SuppressWarnings("unchecked") + public void testChangeDeliveredDuringDispose() throws Exception { + final AtomicBoolean changeDelivered = new AtomicBoolean(false); + when(bundleContext.registerService(eq(ResourceChangeListener.class), any(ResourceChangeListener.class), any())) + .thenAnswer((Answer<ServiceRegistration<ResourceChangeListener>>) invocation -> { + final ResourceChangeListener listener = invocation.getArgument(1); + final ServiceRegistration<ResourceChangeListener> registration = + Mockito.mock(ServiceRegistration.class); + Mockito.doAnswer(unregisterInvocation -> { + listener.onChange(List.of( + new ResourceChange(ResourceChange.ChangeType.ADDED, "/resource", false))); + changeDelivered.set(true); + return null; + }) + .when(registration) + .unregister(); + return registration; + }); + + final MapEntries entries = new MapEntries( + resourceResolverFactory, bundleContext, eventAdmin, stringInterpolationProvider, Optional.empty()); + entries.dispose(); + assertTrue("change should have been delivered during listener unregistration", changeDelivered.get()); + } + @Test public void testTimingFormatter() { assertEquals(
