You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ro...@apache.org on 2026/07/28 07:58:23 UTC
(sling-org-apache-sling-resourceresolver) 01/01: SLING-13236: MapEntries: NPE in onChange when a resource change is delivered during construction or disposal (backport to 1.x)
This is an automated email from the ASF dual-hosted git repository.
royteeuwen pushed a commit to branch backport/SLING-13236-1.x
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git
commit 8cd6cf8b9bfc9799dbc978fefdfd872ef2c52dd1
Author: Roy Teeuwen <ro...@apache.org>
AuthorDate: Tue Jul 28 09:23:09 2026 +0200
SLING-13236: 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(