From 882bc6ee1e5c41d7418535e8e6869caf8733c419 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Thu, 24 Sep 2026 09:42:08 +0300 Subject: [PATCH] [#228] Create the shared HttpContext lazily and dump threads on a start-up hang ServletRegistrationSingleton.activate() called WebContainer.createDefaultSharedHttpContext(), which pax-web runs on its single configuration thread. SCR holds the component's state lock during activate(), while the configuration thread, registering WebContainer, waits for that same lock to activate ServletComponent. SCR broke the cycle after ds.lock.timeout with "Could not obtain lock", and start-up never reached "OpenIDM ready". Create the context on first use without holding a lock, and in CI take a jcmd thread dump when OpenIDM does not become ready within the timeout. Fixes #228 --- .github/workflows/build.yml | 18 +++- .../impl/ServletRegistrationSingleton.java | 27 ++++-- .../ServletRegistrationSingletonTest.java | 86 +++++++++++++++++++ 3 files changed, 124 insertions(+), 7 deletions(-) create mode 100644 openidm-servlet-registrator/src/test/java/org/forgerock/openidm/servletregistration/impl/ServletRegistrationSingletonTest.java diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index bfda1ce5f..10c27101b 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -68,7 +68,14 @@ jobs: fi unzip openidm-zip/target/openidm-*.zip openidm/startup.sh & - timeout 3m bash -c 'until grep -q "OpenIDM ready" openidm/logs/openidm0.log.0 ; do sleep 5; done' || cat openidm/logs/openidm0.log.0 + timeout 3m bash -c 'until grep -q "OpenIDM ready" openidm/logs/openidm0.log.0 ; do sleep 5; done' || { + cat openidm/logs/openidm0.log.0 + # A start-up that never gets ready is a hang (#228): dump the threads before failing. + for pid in $(pgrep -f org.forgerock.commons.launcher.Main); do + jcmd "$pid" Thread.print -l > openidm/logs/threaddump-$pid.txt 2>&1 || true + cat openidm/logs/threaddump-$pid.txt + done + } grep -q "OpenIDM ready" openidm/logs/openidm0.log.0 ! grep -E "ERROR|SEVERE|Exception|Throwable" openidm/logs/openidm0.log.0 - name: Test on Windows @@ -185,7 +192,14 @@ jobs: ARGS="-p ${{ matrix.samples }}" fi OPENIDM_OPTS="$OPTS" openidm/startup.sh $ARGS & - timeout 3m bash -c 'until grep -q "OpenIDM ready" openidm/logs/openidm0.log.0 ; do sleep 5; done' || cat openidm/logs/openidm0.log.0 + timeout 3m bash -c 'until grep -q "OpenIDM ready" openidm/logs/openidm0.log.0 ; do sleep 5; done' || { + cat openidm/logs/openidm0.log.0 + # A start-up that never gets ready is a hang (#228): dump the threads before failing. + for pid in $(pgrep -f org.forgerock.commons.launcher.Main); do + jcmd "$pid" Thread.print -l > openidm/logs/threaddump-$pid.txt 2>&1 || true + cat openidm/logs/threaddump-$pid.txt + done + } grep -q "OpenIDM ready" openidm/logs/openidm0.log.0 # Allow-list of documented, expected log-noise per sample. The # usecase1 walk-through explicitly relies on three iterative recon diff --git a/openidm-servlet-registrator/src/main/java/org/forgerock/openidm/servletregistration/impl/ServletRegistrationSingleton.java b/openidm-servlet-registrator/src/main/java/org/forgerock/openidm/servletregistration/impl/ServletRegistrationSingleton.java index 6fcdecc6f..730963746 100644 --- a/openidm-servlet-registrator/src/main/java/org/forgerock/openidm/servletregistration/impl/ServletRegistrationSingleton.java +++ b/openidm-servlet-registrator/src/main/java/org/forgerock/openidm/servletregistration/impl/ServletRegistrationSingleton.java @@ -43,6 +43,7 @@ import java.util.Hashtable; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; import jakarta.servlet.Filter; import jakarta.servlet.Servlet; @@ -113,7 +114,15 @@ private static String[] getDefaultServletUrlPatterns() { @Reference private WebContainer webContainer; - private HttpContext sharedContext; + /** + * Created on first use rather than in {@link #activate}: SCR holds this component's state lock + * during activation, and a {@link WebContainer} call from any thread other than the pax-web + * configuration thread waits for that thread. The configuration thread in turn waits for this + * state lock when it registers the {@link WebContainer} service and activates the components + * that depend on {@link ServletRegistration}. No lock is held while the context is created, for + * the same reason; a concurrently created duplicate is discarded. + */ + private final AtomicReference sharedContext = new AtomicReference<>(); private List filters = new ArrayList(); @@ -127,7 +136,15 @@ private static String[] getDefaultServletUrlPatterns() { @Activate public void activate(ComponentContext context) { bundleContext = context.getBundleContext(); - sharedContext = webContainer.createDefaultSharedHttpContext(); + } + + private HttpContext sharedContext() { + HttpContext context = sharedContext.get(); + if (context == null) { + sharedContext.compareAndSet(null, webContainer.createDefaultSharedHttpContext()); + context = sharedContext.get(); + } + return context; } /** @@ -145,7 +162,7 @@ public void deactivate(ComponentContext context) { */ @SuppressWarnings("rawtypes") public void registerServlet(String alias, Servlet servlet, Dictionary initparams) throws ServletException, NamespaceException { - webContainer.registerServlet(alias, servlet, initparams, sharedContext); + webContainer.registerServlet(alias, servlet, initparams, sharedContext()); } /** @@ -157,7 +174,7 @@ public void unregisterServlet(Servlet servlet) { @Override public HttpContext getContext() { - return sharedContext; + return sharedContext(); } /** @@ -263,7 +280,7 @@ public URL apply(JsonValue jsonValue) throws JsonValueException { urlPatterns.toArray(new String[urlPatterns.size()]), servletNames.toArray(new String[servletNames.size()]), new Hashtable<>(initParams), - sharedContext); + sharedContext()); return proxiedFilter; } diff --git a/openidm-servlet-registrator/src/test/java/org/forgerock/openidm/servletregistration/impl/ServletRegistrationSingletonTest.java b/openidm-servlet-registrator/src/test/java/org/forgerock/openidm/servletregistration/impl/ServletRegistrationSingletonTest.java new file mode 100644 index 000000000..5fe4384cc --- /dev/null +++ b/openidm-servlet-registrator/src/test/java/org/forgerock/openidm/servletregistration/impl/ServletRegistrationSingletonTest.java @@ -0,0 +1,86 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ + +package org.forgerock.openidm.servletregistration.impl; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; +import static org.testng.Assert.assertSame; + +import java.lang.reflect.Field; +import java.util.Hashtable; + +import jakarta.servlet.Servlet; + +import org.ops4j.pax.web.service.MultiBundleWebContainerContext; +import org.ops4j.pax.web.service.WebContainer; +import org.osgi.framework.BundleContext; +import org.osgi.service.component.ComponentContext; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +public class ServletRegistrationSingletonTest { + + private WebContainer webContainer; + private MultiBundleWebContainerContext sharedContext; + private ServletRegistrationSingleton registration; + + @BeforeMethod + public void setUp() throws Exception { + webContainer = mock(WebContainer.class); + sharedContext = mock(MultiBundleWebContainerContext.class); + when(webContainer.createDefaultSharedHttpContext()).thenReturn(sharedContext); + + registration = new ServletRegistrationSingleton(); + Field field = ServletRegistrationSingleton.class.getDeclaredField("webContainer"); + field.setAccessible(true); + field.set(registration, webContainer); + } + + /** + * SCR holds the component's state lock while activate() runs. A WebContainer call made there is + * executed on the single pax-web configuration thread, which may itself be waiting for that + * state lock while it registers the WebContainer service (#228), so activate() must not call it. + */ + @Test + public void activateDoesNotCallWebContainer() { + registration.activate(componentContext()); + + verifyZeroInteractions(webContainer); + } + + @Test + public void sharedContextIsCreatedOnceOnFirstUse() throws Exception { + registration.activate(componentContext()); + Servlet servlet = mock(Servlet.class); + Hashtable params = new Hashtable<>(); + + assertSame(registration.getContext(), sharedContext); + registration.registerServlet("/openidm", servlet, params); + + verify(webContainer, times(1)).createDefaultSharedHttpContext(); + verify(webContainer).registerServlet("/openidm", servlet, params, sharedContext); + } + + private static ComponentContext componentContext() { + ComponentContext context = mock(ComponentContext.class); + when(context.getBundleContext()).thenReturn(mock(BundleContext.class)); + return context; + } +}