This is an automated email from the ASF dual-hosted git repository.
chetanm pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-commons-log.git
The following commit(s) were added to refs/heads/master by this push:
new 3271098 SLING-7239 - LogbackManager may miss out some OSGi config at
time of startup
3271098 is described below
commit 3271098b98cc661a3ee3cdb72aa1ef653b5d873d
Author: Chetan Mehrotra <[email protected]>
AuthorDate: Mon Nov 27 11:49:50 2017 +0530
SLING-7239 - LogbackManager may miss out some OSGi config at time of startup
---
.../log/logback/internal/LogConfigManager.java | 21 +++
.../log/logback/internal/LogbackManager.java | 3 +
.../ITConfigRaceCondition_SLING_7239.java | 154 +++++++++++++++++++++
.../log/logback/integration/LogTestBase.java | 6 +-
4 files changed, 183 insertions(+), 1 deletion(-)
diff --git
a/src/main/java/org/apache/sling/commons/log/logback/internal/LogConfigManager.java
b/src/main/java/org/apache/sling/commons/log/logback/internal/LogConfigManager.java
index a001a53..ee8c9e7 100644
---
a/src/main/java/org/apache/sling/commons/log/logback/internal/LogConfigManager.java
+++
b/src/main/java/org/apache/sling/commons/log/logback/internal/LogConfigManager.java
@@ -99,6 +99,8 @@ public class LogConfigManager implements
LogbackResetListener, LogConfig.LogWrit
public static final String DEFAULT_CONSOLE_APPENDER_NAME =
"org.apache.sling.commons.log.CONSOLE";
+ private static final String CONFIG_PID_SET =
"org.apache.sling.commons.log.ConfigPids";
+
private final LoggerContext loggerContext;
private final ContextUtil contextUtil;
@@ -274,7 +276,9 @@ public class LogConfigManager implements
LogbackResetListener, LogConfig.LogWrit
Map<Appender, LoggerSpecificEncoder> encoders = new HashMap<Appender,
LoggerSpecificEncoder>();
+ Set<String> configPids = new HashSet<>();
for (LogConfig config : getLogConfigs()) {
+ configPids.add(config.getConfigPid());
Appender<ILoggingEvent> appender = null;
if (config.isAppenderDefined()) {
LogWriter lw = config.getLogWriter();
@@ -318,6 +322,9 @@ public class LogConfigManager implements
LogbackResetListener, LogConfig.LogWrit
}
}
}
+
+ //Record the config pids which have been picked up in this reset cycle
+ context.putObject(CONFIG_PID_SET, configPids);
}
@@ -601,6 +608,20 @@ public class LogConfigManager implements
LogbackResetListener, LogConfig.LogWrit
}
}
+ public void checkForNewConfigsWhileStarting(LoggerContext context){
+ Set<String> configPids = (Set<String>)
context.getObject(CONFIG_PID_SET);
+ if (configPids == null) {
+ contextUtil.addWarn("Did not find any configPid set");
+ return;
+ }
+ if (!configPids.equals(configByPid.keySet())) {
+ contextUtil.addInfo("Config change detected post start. Scheduling
config reload");
+ logbackManager.configChanged();
+ } else {
+ contextUtil.addInfo("Configured the Logback with " +
configPids.size() + " configs");
+ }
+ }
+
public boolean isPackagingDataEnabled() {
return packagingDataEnabled;
}
diff --git
a/src/main/java/org/apache/sling/commons/log/logback/internal/LogbackManager.java
b/src/main/java/org/apache/sling/commons/log/logback/internal/LogbackManager.java
index 582d487..e63c6e5 100644
---
a/src/main/java/org/apache/sling/commons/log/logback/internal/LogbackManager.java
+++
b/src/main/java/org/apache/sling/commons/log/logback/internal/LogbackManager.java
@@ -181,6 +181,9 @@ public class LogbackManager extends LoggerContextAwareBase {
// now open the gate for regular configuration
started = true;
+
+ //Now check once if any other config was added while we were starting
+ logConfigManager.checkForNewConfigsWhileStarting(getLoggerContext());
}
public void shutdown() {
diff --git
a/src/test/java/org/apache/sling/commons/log/logback/integration/ITConfigRaceCondition_SLING_7239.java
b/src/test/java/org/apache/sling/commons/log/logback/integration/ITConfigRaceCondition_SLING_7239.java
new file mode 100644
index 0000000..674410f
--- /dev/null
+++
b/src/test/java/org/apache/sling/commons/log/logback/integration/ITConfigRaceCondition_SLING_7239.java
@@ -0,0 +1,154 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.sling.commons.log.logback.integration;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Dictionary;
+import java.util.Hashtable;
+import java.util.Random;
+import java.util.concurrent.CountDownLatch;
+
+import javax.inject.Inject;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.ops4j.pax.exam.Option;
+import org.ops4j.pax.exam.junit.PaxExam;
+import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy;
+import org.ops4j.pax.exam.spi.reactors.PerClass;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.BundleEvent;
+import org.osgi.framework.BundleListener;
+import org.osgi.service.cm.Configuration;
+import org.osgi.service.cm.ConfigurationAdmin;
+
+import static java.lang.String.format;
+import static
org.apache.sling.commons.log.logback.integration.ITConfigAdminSupport.FACTORY_PID_CONFIGS;
+import static
org.apache.sling.commons.log.logback.integration.ITConfigAdminSupport.LOG_FILE;
+import static
org.apache.sling.commons.log.logback.integration.ITConfigAdminSupport.LOG_LEVEL;
+import static
org.apache.sling.commons.log.logback.integration.ITConfigAdminSupport.LOG_LOGGERS;
+import static org.junit.Assert.fail;
+import static org.ops4j.pax.exam.CoreOptions.composite;
+import static org.ops4j.pax.exam.CoreOptions.mavenBundle;
+
+@RunWith(PaxExam.class)
+@ExamReactorStrategy(PerClass.class)
+public class ITConfigRaceCondition_SLING_7239 extends LogTestBase {
+ private static final String LOG_BUNDLE_NAME =
"org.apache.sling.commons.log";
+
+ @Rule
+ public final TemporaryFolder tmpFolder = new TemporaryFolder(new
File("target"));
+
+ @Inject
+ private ConfigurationAdmin ca;
+
+ @Inject
+ private BundleContext bundleContext;
+
+ private Random rnd = new Random();
+
+ @Override
+ protected Option addExtraOptions() {
+ return composite(configAdmin(), mavenBundle("commons-io",
"commons-io").versionAsInProject());
+ }
+
+ @Override
+ protected boolean shouldStartLogBundle() {
+ return false;
+ }
+
+ @Test
+ public void multipleLogConfigs() throws Exception {
+ int configCount = 200 + rnd.nextInt(100);
+
+ //1. Create lots of log configs. This would create a situation where
+ //only few of the config get picked up at time of activation
+ createLogConfigs(configCount);
+
+ //2. Register listener to get notified when the activator is done
+ CountDownLatch startLatch = new CountDownLatch(1);
+ BundleListener listener = e -> {
+ if (e.getBundle().getSymbolicName().equals(LOG_BUNDLE_NAME)
+ && e.getType() == BundleEvent.STARTED) {
+ startLatch.countDown();
+ }
+ };
+
+ bundleContext.addBundleListener(listener);
+
+ Bundle logb = getBundle(LOG_BUNDLE_NAME);
+ logb.start();
+
+ //3. Wait for activator to complete
+ startLatch.await();
+
+ //Now check by polling that number of log file created in log dir
match the
+ //actual number of configs created
+ new RetryLoop(new RetryLoop.Condition() {
+ @Override
+ public String getDescription() {
+ return format("Expected log file count [%d], Found [%d]",
configCount, getLogFileCount());
+ }
+
+ @Override
+ public boolean isTrue() throws Exception {
+ return configCount == getLogFileCount();
+ }
+ }, 15, 100);
+
+ }
+
+ private int getLogFileCount() {
+ return tmpFolder.getRoot().listFiles().length;
+ }
+
+ private Bundle getBundle(String bundleName) {
+ for (Bundle b : bundleContext.getBundles()) {
+ if (bundleName.equals(b.getSymbolicName())) {
+ return b;
+ }
+ }
+ fail("Not able find bundle " + bundleName);
+ return null;
+ }
+
+ private void createLogConfigs(int count) throws IOException {
+ for (int i = 0; i < count; i++) {
+ createLogConfig(i);
+ }
+ }
+
+ private void createLogConfig(int index) throws IOException {
+ Configuration config =
ca.createFactoryConfiguration(FACTORY_PID_CONFIGS, null);
+ Dictionary<String, Object> p = new Hashtable<String, Object>();
+ p.put(LOG_LOGGERS, new String[]{
+ "foo.bar." + index
+ });
+ p.put(LOG_LEVEL, "DEBUG");
+
+ File logFile = new File(tmpFolder.getRoot(), "error-" + index +
".log");
+ p.put(LOG_FILE, logFile.getAbsolutePath());
+ config.update(p);
+ }
+}
diff --git
a/src/test/java/org/apache/sling/commons/log/logback/integration/LogTestBase.java
b/src/test/java/org/apache/sling/commons/log/logback/integration/LogTestBase.java
index 196006f..239ae6b 100644
---
a/src/test/java/org/apache/sling/commons/log/logback/integration/LogTestBase.java
+++
b/src/test/java/org/apache/sling/commons/log/logback/integration/LogTestBase.java
@@ -84,7 +84,7 @@ public abstract class LogTestBase {
}
return options(
// the current project (the bundle under test)
- CoreOptions.bundle(bundleFile.toURI().toString()),
+
CoreOptions.bundle(bundleFile.toURI().toString()).start(shouldStartLogBundle()),
mavenBundle("org.slf4j", "slf4j-api").versionAsInProject(),
addPaxExamSpecificOptions(),
addCodeCoverageOption(), addDebugOptions(), addExtraOptions(),
addDefaultOptions());
}
@@ -110,6 +110,10 @@ public abstract class LogTestBase {
return null;
}
+ protected boolean shouldStartLogBundle(){
+ return true;
+ }
+
private static Option addCodeCoverageOption() {
String coverageCommand = System.getProperty(COVERAGE_COMMAND);
if (coverageCommand != null && !coverageCommand.isEmpty()) {
--
To stop receiving notification emails like this one, please contact
['"[email protected]" <[email protected]>'].
