Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[JENKINS-46386] Make SystemProperties safe to call from agent JVMs.
  • Loading branch information
jglick committed Mar 21, 2018
1 parent ca61ed7 commit 7948315
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 30 deletions.
64 changes: 36 additions & 28 deletions core/src/main/java/jenkins/util/SystemProperties.java
Expand Up @@ -24,7 +24,6 @@
package jenkins.util;

import edu.umd.cs.findbugs.annotations.CheckForNull;
import edu.umd.cs.findbugs.annotations.Nullable;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import hudson.EnvVars;
import java.util.logging.Level;
Expand Down Expand Up @@ -67,34 +66,40 @@
*/
//TODO: Define a correct design of this engine later. Should be accessible in libs (remoting, stapler) and Jenkins modules too
@Restricted(NoExternalUse.class)
public class SystemProperties implements ServletContextListener, OnMaster {
// this class implements ServletContextListener and is declared in WEB-INF/web.xml
public class SystemProperties {

/**
* The ServletContext to get the "init" parameters from.
*/
@CheckForNull
private static ServletContext theContext;
// declared in WEB-INF/web.xml
public static final class Listener implements ServletContextListener, OnMaster {

/**
* The ServletContext to get the "init" parameters from.
*/
@CheckForNull
private static ServletContext theContext;

/**
* Called by the servlet container to initialize the {@link ServletContext}.
*/
@Override
@SuppressFBWarnings(value = "ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD",
justification = "Currently Jenkins instance may have one ond only one context")
public void contextInitialized(ServletContextEvent event) {
theContext = event.getServletContext();
}

@Override
public void contextDestroyed(ServletContextEvent event) {
theContext = null;
}

}

/**
* Logger.
*/
private static final Logger LOGGER = Logger.getLogger(SystemProperties.class.getName());

/**
* Public for the servlet container.
*/
public SystemProperties() {}

/**
* Called by the servlet container to initialize the {@link ServletContext}.
*/
@Override
@SuppressFBWarnings(value = "ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD",
justification = "Currently Jenkins instance may have one ond only one context")
public void contextInitialized(ServletContextEvent event) {
theContext = event.getServletContext();
}
private SystemProperties() {}

/**
* Gets the system property indicated by the specified key.
Expand Down Expand Up @@ -373,9 +378,16 @@ public static Long getLong(String name, Long def, Level logLevel) {

@CheckForNull
private static String tryGetValueFromContext(String key) {
if (StringUtils.isNotBlank(key) && theContext != null) {
if (!JenkinsJVM.isJenkinsJVM()) {
return null;
}
return doTryGetValueFromContext(key);
}

private static String doTryGetValueFromContext(String key) {
if (StringUtils.isNotBlank(key) && Listener.theContext != null) {
try {
String value = theContext.getInitParameter(key);
String value = Listener.theContext.getInitParameter(key);
if (value != null) {
return value;
}
Expand All @@ -387,8 +399,4 @@ private static String tryGetValueFromContext(String key) {
return null;
}

@Override
public void contextDestroyed(ServletContextEvent event) {
// nothing to do
}
}
2 changes: 1 addition & 1 deletion test/src/test/java/jenkins/util/SystemPropertiesTest.java
Expand Up @@ -46,7 +46,7 @@ public class SystemPropertiesTest {

@Before
public void setUp() {
new SystemProperties().contextInitialized(new ServletContextEvent(j.jenkins.servletContext));
new SystemProperties.Listener().contextInitialized(new ServletContextEvent(j.jenkins.servletContext));
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion war/src/main/webapp/WEB-INF/web.xml
Expand Up @@ -153,7 +153,7 @@ THE SOFTWARE.

<listener>
<!-- Must be before WebAppMain in order to initialize the context before the first use of this class. -->
<listener-class>jenkins.util.SystemProperties</listener-class>
<listener-class>jenkins.util.SystemProperties$Listener</listener-class>
</listener>
<listener>
<listener-class>hudson.WebAppMain</listener-class>
Expand Down

0 comments on commit 7948315

Please sign in to comment.