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 09:25:32 UTC
(sling-org-apache-sling-resourceresolver) branch feature/SLING-13274-1.x created (now 5da8528f)
This is an automated email from the ASF dual-hosted git repository.
royteeuwen pushed a change to branch feature/SLING-13274-1.x
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git
at 5da8528f SLING-13274: MapEntries: NPE in onChange when a resource change is delivered during construction or disposal (backport to 1.x)
This branch includes the following new commits:
new 5da8528f SLING-13274: MapEntries: NPE in onChange when a resource change is delivered during construction or disposal (backport to 1.x)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(sling-org-apache-sling-resourceresolver) 01/01: SLING-13274: MapEntries: NPE in onChange when a resource change is delivered during construction or disposal (backport to 1.x)
Posted by ro...@apache.org.
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 <ro...@apache.org>
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(