Spring 33450 - Spring shutdown fails due to IllegalStateException (#2062)

Author: rgoers

log4j-api-test/src/test/java/org/apache/logging/log4j/util/PropertiesUtilTest.java

@@ -188,6 +188,23 @@ public void testPublish() {
         assertEquals("Log4j", value);
     }
 
+    @Test
+    @ResourceLock(Resources.SYSTEM_PROPERTIES)
+    public void testBadPropertysource() {
+        final String key1 = "testKey";
+        System.getProperties().put(key1, "test");
+        final PropertiesUtil util = new PropertiesUtil(new Properties());
+        ErrorPropertySource source = new ErrorPropertySource();
+        util.addPropertySource(source);
+        try {
+            assertEquals("test", util.getStringProperty(key1));
+            assertTrue(source.exceptionThrown);
+        } finally {
+            util.removePropertySource(source);
+            System.getProperties().remove(key1);
+        }
+    }
+
     private static final String[][] data = {
         {null, "org.apache.logging.log4j.level"},
         {null, "Log4jAnotherProperty"},
@@ -231,4 +248,25 @@ public void testLog4jProperty() {
         final PropertiesUtil util = new PropertiesUtil(props);
         assertEquals(correct, util.getStringProperty(correct));
     }
+
+    private class ErrorPropertySource implements PropertySource {
+        public boolean exceptionThrown = false;
+
+        @Override
+        public int getPriority() {
+            return Integer.MIN_VALUE;
+        }
+
+        @Override
+        public String getProperty(String key) {
+            exceptionThrown = true;
+            throw new InstantiationError("Test");
+        }
+
+        @Override
+        public boolean containsProperty(String key) {
+            exceptionThrown = true;
+            throw new InstantiationError("Test");
+        }
+    }
 }

log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesUtil.java

@@ -139,15 +139,27 @@ public static PropertiesUtil getProperties() {
     }
 
     /**
-     * Allows a PropertySource to be added after PropertiesUtil has been created.
-     * @param propertySource the PropertySource to add.
+     * Allows a {@link PropertySource} to be added after PropertiesUtil has been created.
+     * @param propertySource the {@code PropertySource} to add.
+     * @since 2.19.0
      */
     public void addPropertySource(final PropertySource propertySource) {
         if (environment != null) {
             environment.addPropertySource(propertySource);
         }
     }
 
+    /**
+     * Removes a {@link PropertySource}.
+     * @param propertySource the {@code PropertySource} to remove.
+     * @since 2.24.0
+     */
+    public void removePropertySource(final PropertySource propertySource) {
+        if (environment != null) {
+            environment.removePropertySource(propertySource);
+        }
+    }
+
     /**
      * Returns {@code true} if the specified property is defined, regardless of its value (it may not have a value).
      *
@@ -529,6 +541,10 @@ public void addPropertySource(final PropertySource propertySource) {
             sources.add(propertySource);
         }
 
+        private void removePropertySource(final PropertySource propertySource) {
+            sources.remove(propertySource);
+        }
+
         private synchronized void reload() {
             literal.clear();
             tokenized.clear();
@@ -540,18 +556,18 @@ private synchronized void reload() {
                 final List<CharSequence> tokens = PropertySource.Util.tokenize(key);
                 final boolean hasTokens = !tokens.isEmpty();
                 sources.forEach(source -> {
-                    if (source.containsProperty(key)) {
-                        final String value = source.getProperty(key);
+                    if (sourceContainsProperty(source, key)) {
+                        final String value = sourceGetProperty(source, key);
                         if (hasTokens) {
                             tokenized.putIfAbsent(tokens, value);
                         }
                     }
                     if (hasTokens) {
                         final String normalKey = Objects.toString(source.getNormalForm(tokens), null);
-                        if (normalKey != null && source.containsProperty(normalKey)) {
-                            literal.putIfAbsent(key, source.getProperty(normalKey));
-                        } else if (source.containsProperty(key)) {
-                            literal.putIfAbsent(key, source.getProperty(key));
+                        if (normalKey != null && sourceContainsProperty(source, normalKey)) {
+                            literal.putIfAbsent(key, sourceGetProperty(source, normalKey));
+                        } else if (sourceContainsProperty(source, key)) {
+                            literal.putIfAbsent(key, sourceGetProperty(source, key));
                         }
                     }
                 });
@@ -567,25 +583,41 @@ private String get(final String key) {
             for (final PropertySource source : sources) {
                 if (hasTokens) {
                     final String normalKey = Objects.toString(source.getNormalForm(tokens), null);
-                    if (normalKey != null && source.containsProperty(normalKey)) {
-                        return source.getProperty(normalKey);
+                    if (normalKey != null && sourceContainsProperty(source, normalKey)) {
+                        return sourceGetProperty(source, normalKey);
                     }
                 }
-                if (source.containsProperty(key)) {
-                    return source.getProperty(key);
+                if (sourceContainsProperty(source, key)) {
+                    return sourceGetProperty(source, key);
                 }
             }
             return tokenized.get(tokens);
         }
 
+        private boolean sourceContainsProperty(final PropertySource source, final String key) {
+            try {
+                return source.containsProperty(key);
+            } catch (final Throwable ex) {
+                return false;
+            }
+        }
+
+        private String sourceGetProperty(final PropertySource source, final String key) {
+            try {
+                return source.getProperty(key);
+            } catch (final Throwable ex) {
+                return null;
+            }
+        }
+
         private boolean containsKey(final String key) {
             final List<CharSequence> tokens = PropertySource.Util.tokenize(key);
             return literal.containsKey(key)
                     || tokenized.containsKey(tokens)
                     || sources.stream().anyMatch(s -> {
                         final CharSequence normalizedKey = s.getNormalForm(tokens);
-                        return s.containsProperty(key)
-                                || (normalizedKey != null && s.containsProperty(normalizedKey.toString()));
+                        return sourceContainsProperty(s, key)
+                                || (normalizedKey != null && sourceContainsProperty(s, normalizedKey.toString()));
                     });
         }
     }

log4j-core/src/main/java/org/apache/logging/log4j/core/jmx/Server.java

@@ -240,12 +240,14 @@ private static ContextSelector getContextSelector() {
      * @param loggerContextName name of the logger context to unregister
      */
     public static void unregisterLoggerContext(final String loggerContextName) {
-        if (isJmxDisabled()) {
-            LOGGER.debug("JMX disabled for Log4j2. Not unregistering MBeans.");
-            return;
+        if (loggerContextName != null) {
+            if (isJmxDisabled()) {
+                LOGGER.debug("JMX disabled for Log4j2. Not unregistering MBeans.");
+                return;
+            }
+            final MBeanServer mbs = ManagementFactory.getPlatformMBeanServer();
+            unregisterLoggerContext(loggerContextName, mbs);
         }
-        final MBeanServer mbs = ManagementFactory.getPlatformMBeanServer();
-        unregisterLoggerContext(loggerContextName, mbs);
     }
 
     /**

src/changelog/.2.x.x/1799_ignore_propertysource_errors.xml

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xmlns="http://logging.apache.org/log4j/changelog"
+       xsi:schemaLocation="http://logging.apache.org/log4j/changelog https://logging.apache.org/log4j/changelog-0.1.3.xsd"
+       type="changed">
+  <issue id="Spring-33450" link="https://github.com/spring-projects/spring-boot/issues/33450"/>
+  <description format="asciidoc">Ignore exceptions thrown by PropertySources.</description>
+</entry>