This is an automated email from the ASF dual-hosted git repository. ghenzler pushed a commit to branch bugfix/FELIX-6815-fix-racecondition in repository https://gitbox.apache.org/repos/asf/felix-dev.git
commit d15ff9a0c561431d03ffeeb6ae846f375d0e3ebe Author: Georg Henzler <[email protected]> AuthorDate: Wed Jan 21 19:17:14 2026 +0100 FELIX-6815 Fix race condition for defaultBaseUrl --- .../felix/hc/generalchecks/HttpRequestsCheck.java | 86 ++++++++++++++------ .../hc/generalchecks/HttpRequestsCheckTest.java | 93 ++++++++++++++++++++++ 2 files changed, 154 insertions(+), 25 deletions(-) diff --git a/healthcheck/generalchecks/src/main/java/org/apache/felix/hc/generalchecks/HttpRequestsCheck.java b/healthcheck/generalchecks/src/main/java/org/apache/felix/hc/generalchecks/HttpRequestsCheck.java index ce8e3996ea..313fc2be85 100644 --- a/healthcheck/generalchecks/src/main/java/org/apache/felix/hc/generalchecks/HttpRequestsCheck.java +++ b/healthcheck/generalchecks/src/main/java/org/apache/felix/hc/generalchecks/HttpRequestsCheck.java @@ -35,6 +35,7 @@ import java.net.URL; import java.util.ArrayList; import java.util.Arrays; import java.util.Base64; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -58,10 +59,15 @@ import org.apache.felix.hc.core.impl.util.lang.StringUtils; import org.apache.felix.hc.generalchecks.util.SimpleConstraintChecker; import org.apache.felix.utils.json.JSONParser; import org.osgi.framework.BundleContext; +import org.osgi.framework.Constants; +import org.osgi.framework.InvalidSyntaxException; import org.osgi.framework.ServiceReference; +import org.osgi.framework.ServiceEvent; +import org.osgi.framework.ServiceListener; import org.osgi.service.component.annotations.Activate; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.ConfigurationPolicy; +import org.osgi.service.component.annotations.Deactivate; import org.osgi.service.metatype.annotations.AttributeDefinition; import org.osgi.service.metatype.annotations.Designate; import org.osgi.service.metatype.annotations.ObjectClassDefinition; @@ -77,6 +83,12 @@ public class HttpRequestsCheck implements HealthCheck { public static final String HC_NAME = "Http Requests"; public static final String HC_LABEL = "Health Check: " + HC_NAME; + private static final String[] HTTP_SERVICES = { + "org.osgi.service.http.HttpService", + "org.osgi.service.http.runtime.HttpServiceRuntime", + "org.osgi.service.servlet.runtime.HttpServiceRuntime" }; + private static final String HTTP_SERVICE_FILTER = "(|(" + Constants.OBJECTCLASS + "=" + String.join(")(" + Constants.OBJECTCLASS + "=", HTTP_SERVICES) + "))"; + @ObjectClassDefinition(name = HC_LABEL, description = "Performs http(s) request(s) and checks the response for return code and optionally checks the response entity") public @interface Config { @@ -132,6 +144,7 @@ public class HttpRequestsCheck implements HealthCheck { private final boolean runInParallel; private volatile String defaultBaseUrl; + private volatile ServiceListener serviceListener; private FormattingResultLog configErrors = new FormattingResultLog(); @@ -144,42 +157,59 @@ public class HttpRequestsCheck implements HealthCheck { this.statusForFailedContraint = config.statusForFailedContraint(); this.runInParallel = config.runInParallel() && requestSpecs.size() > 1; - LOG.debug("Activated Requests HC: {}", requestSpecs); + this.registerServiceListener(); this.setupDefaultBaseUrl(); + LOG.debug("Activated Requests HC: {}, defaultBaseUrl: {}, runInParallel: {}, statusForFailedContraint: {}", requestSpecs, defaultBaseUrl, runInParallel, statusForFailedContraint); + } + + @Deactivate + public void deactivate() { + if (this.serviceListener != null) { + this.bundleContext.removeServiceListener(this.serviceListener); + } + } + + String getDefaultBaseUrl() { + return defaultBaseUrl; + } + + private void registerServiceListener() { + try { + this.serviceListener = new ServiceListener() { + @Override + public void serviceChanged(ServiceEvent event) { + setupDefaultBaseUrl(); + LOG.debug("Updated defaultBaseUrl: {}", defaultBaseUrl); + } + }; + this.bundleContext.addServiceListener(this.serviceListener, HTTP_SERVICE_FILTER); + } catch (InvalidSyntaxException e) { + LOG.warn("Invalid service filter {}", HTTP_SERVICE_FILTER, e); + } } private void setupDefaultBaseUrl() { - // no need to synchronize - if ( this.defaultBaseUrl == null ) { - // check the properties for these services - final String[] services = {"org.osgi.service.http.HttpService", - "org.osgi.service.http.runtime.HttpServiceRuntime", - "org.osgi.service.servlet.runtime.HttpServiceRuntime"}; - for(final String service : services) { - final ServiceReference<?> ref = this.bundleContext.getServiceReference(service); - if ( ref != null ) { - boolean isHttp = Boolean.parseBoolean(String.valueOf(ref.getProperty("org.apache.felix.http.enable"))); - boolean isHttps = Boolean.parseBoolean(String.valueOf(ref.getProperty("org.apache.felix.https.enable"))); - if (isHttps) { - defaultBaseUrl = "http://localhost:"+ref.getProperty("org.osgi.service.https.port"); - } else if (isHttp) { - defaultBaseUrl = "http://localhost:"+ref.getProperty("org.osgi.service.http.port"); - } - if ( this.defaultBaseUrl != null ) { - break; - } + String resolvedBaseUrl = null; + for (final String service : HTTP_SERVICES) { + final ServiceReference<?> ref = this.bundleContext.getServiceReference(service); + if (ref != null) { + boolean isHttp = Boolean.parseBoolean(String.valueOf(ref.getProperty("org.apache.felix.http.enable"))); + boolean isHttps = Boolean.parseBoolean(String.valueOf(ref.getProperty("org.apache.felix.https.enable"))); + if (isHttps) { + resolvedBaseUrl = "http://localhost:"; + ref.getProperty("org.osgi.service.https.port"); + } else if (isHttp) { + resolvedBaseUrl = "http://localhost:"; + ref.getProperty("org.osgi.service.http.port"); + } + if (resolvedBaseUrl != null) { + break; } - } - if ( this.defaultBaseUrl == null ) { - this.defaultBaseUrl = "http://localhost:8080";; - LOG.debug("Default BaseURL: {}", defaultBaseUrl); } } + this.defaultBaseUrl = resolvedBaseUrl; } @Override public Result execute() { - this.setupDefaultBaseUrl(); FormattingResultLog overallLog = new FormattingResultLog(); @@ -363,6 +393,11 @@ public class HttpRequestsCheck implements HealthCheck { public FormattingResultLog check(String defaultBaseUrl, int connectTimeoutInMs, int readTimeoutInMs, Result.Status statusForFailedContraint, boolean showTiming) { FormattingResultLog log = new FormattingResultLog(); + if(url.startsWith("/") && (defaultBaseUrl == null || defaultBaseUrl.isEmpty())) { + log.temporarilyUnavailable("Skipping local path {} (HttpService is not available)", url); + return log; + } + String urlWithUser = user!=null ? user + " @ " + url: url; log.debug("Checking {}", urlWithUser); log.debug(" configured headers {}", headers.keySet()); @@ -399,6 +434,7 @@ public class HttpRequestsCheck implements HealthCheck { URL effectiveUrl; if(url.startsWith("/")) { effectiveUrl = new URL(defaultBaseUrl + url); + log.debug("Effective URL: {}", effectiveUrl); } else { effectiveUrl = new URL(url); } diff --git a/healthcheck/generalchecks/src/test/java/org/apache/felix/hc/generalchecks/HttpRequestsCheckTest.java b/healthcheck/generalchecks/src/test/java/org/apache/felix/hc/generalchecks/HttpRequestsCheckTest.java index ee281bed2a..3b37a771b3 100644 --- a/healthcheck/generalchecks/src/test/java/org/apache/felix/hc/generalchecks/HttpRequestsCheckTest.java +++ b/healthcheck/generalchecks/src/test/java/org/apache/felix/hc/generalchecks/HttpRequestsCheckTest.java @@ -36,7 +36,12 @@ import org.apache.felix.hc.api.ResultLog.Entry; import org.apache.felix.hc.generalchecks.HttpRequestsCheck.RequestSpec; import org.apache.felix.hc.generalchecks.HttpRequestsCheck.ResponseCheck.ResponseCheckResult; import org.junit.Test; +import org.mockito.ArgumentCaptor; import org.mockito.Mockito; +import org.osgi.framework.BundleContext; +import org.osgi.framework.ServiceEvent; +import org.osgi.framework.ServiceListener; +import org.osgi.framework.ServiceReference; public class HttpRequestsCheckTest { @@ -175,5 +180,93 @@ public class HttpRequestsCheckTest { assertArrayEquals(new String[] {"normal1", "\"one two three\"", "normal2", "'one two three'", "-p", "--words", "\"w1 w2 w3\""}, args); } + @Test + public void testDefaultBaseUrlUpdatesOnServiceEvent() throws Exception { + BundleContext bundleContext = Mockito.mock(BundleContext.class); + ServiceReference httpRef = Mockito.mock(ServiceReference.class); + + Mockito.when(bundleContext.getServiceReference("org.osgi.service.http.HttpService")) + .thenReturn((ServiceReference) null, httpRef); + Mockito.when(bundleContext.getServiceReference("org.osgi.service.http.runtime.HttpServiceRuntime")).thenReturn(null); + Mockito.when(bundleContext.getServiceReference("org.osgi.service.servlet.runtime.HttpServiceRuntime")).thenReturn(null); + Mockito.when(httpRef.getProperty("org.apache.felix.http.enable")).thenReturn(true); + Mockito.when(httpRef.getProperty("org.osgi.service.http.port")).thenReturn("9090"); + + ArgumentCaptor<ServiceListener> listenerCaptor = ArgumentCaptor.forClass(ServiceListener.class); + Mockito.doNothing().when(bundleContext).addServiceListener(listenerCaptor.capture(), anyString()); + + HttpRequestsCheck httpRequestsCheck = new HttpRequestsCheck(createConfig(), bundleContext); + + assertNull(httpRequestsCheck.getDefaultBaseUrl()); + + listenerCaptor.getValue().serviceChanged(new ServiceEvent(ServiceEvent.REGISTERED, httpRef)); + + assertEquals("http://localhost:9090";, httpRequestsCheck.getDefaultBaseUrl()); + } + + @Test + public void testRelativeUrlWithoutHttpServiceReturnsUnavailableLog() throws Exception { + HttpRequestsCheck.RequestSpec requestSpec = new HttpRequestsCheck.RequestSpec("/path/to/page.html"); + FormattingResultLog resultLog = requestSpec.check(null, 1000, 1000, Result.Status.WARN, false); + + Iterator<Entry> entryIt = resultLog.iterator(); + Entry lastEntry = null; + while(entryIt.hasNext()) { + lastEntry = entryIt.next(); + } + + assertEquals(Result.Status.TEMPORARILY_UNAVAILABLE, lastEntry.getStatus()); + assertThat(lastEntry.getMessage(), containsString("HttpService is not available")); + } + + private HttpRequestsCheck.Config createConfig() { + return new HttpRequestsCheck.Config() { + @Override + public String hc_name() { + return HttpRequestsCheck.HC_NAME; + } + + @Override + public String[] hc_tags() { + return new String[0]; + } + + @Override + public String[] requests() { + return new String[] { "/path/to/page.html" }; + } + + @Override + public int connectTimeoutInMs() { + return 7000; + } + + @Override + public int readTimeoutInMs() { + return 7000; + } + + @Override + public Result.Status statusForFailedContraint() { + return Result.Status.WARN; + } + + @Override + public boolean runInParallel() { + return false; + } + + @Override + public String webconsole_configurationFactory_nameHint() { + return "{hc.name}: {requests}"; + } + + @Override + public Class<? extends java.lang.annotation.Annotation> annotationType() { + return HttpRequestsCheck.Config.class; + } + }; + } + }
