You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by ch...@apache.org on 2017/01/20 10:36:52 UTC
svn commit: r1779601 - in /felix/trunk/jaas/src:
main/java/org/apache/felix/jaas/internal/ConfigSpiOsgi.java
test/java/org/apache/felix/jaas/internal/ITConcurrentLoginModuleFactoryTest.java
Author: chetanm
Date: Fri Jan 20 10:36:52 2017
New Revision: 1779601
URL: http://svn.apache.org/viewvc?rev=1779601&view=rev
Log:
FELIX-5505 - ConfigSpiOSGi may miss out on registering some LoginModuleFactory due to race condition
Modified:
felix/trunk/jaas/src/main/java/org/apache/felix/jaas/internal/ConfigSpiOsgi.java
felix/trunk/jaas/src/test/java/org/apache/felix/jaas/internal/ITConcurrentLoginModuleFactoryTest.java
Modified: felix/trunk/jaas/src/main/java/org/apache/felix/jaas/internal/ConfigSpiOsgi.java
URL: http://svn.apache.org/viewvc/felix/trunk/jaas/src/main/java/org/apache/felix/jaas/internal/ConfigSpiOsgi.java?rev=1779601&r1=1779600&r2=1779601&view=diff
==============================================================================
--- felix/trunk/jaas/src/main/java/org/apache/felix/jaas/internal/ConfigSpiOsgi.java (original)
+++ felix/trunk/jaas/src/main/java/org/apache/felix/jaas/internal/ConfigSpiOsgi.java Fri Jan 20 10:36:52 2017
@@ -63,12 +63,12 @@ public class ConfigSpiOsgi extends Confi
public static final String SERVICE_PID = "org.apache.felix.jaas.ConfigurationSpi";
- private static enum GlobalConfigurationPolicy
+ private enum GlobalConfigurationPolicy
{
DEFAULT, REPLACE, PROXY
}
- private Map<String, Realm> configs = Collections.emptyMap();
+ private volatile Map<String, Realm> configs = Collections.emptyMap();
private final Logger log;
@@ -206,24 +206,21 @@ public class ConfigSpiOsgi extends Confi
realm.afterPropertiesSet();
}
+ this.configs = Collections.unmodifiableMap(realmToConfigMap);
+ }
+
+ private void registerSpiWithOSGi()
+ {
//We also register the Spi with OSGI SR if any configuration is available
//This would allow any client component to determine when it should start
//and use the config
- if (!realmToConfigMap.isEmpty() && spiReg == null)
+ if (spiReg == null && configs != null && !configs.isEmpty())
{
Properties props = new Properties();
props.setProperty("providerName", "felix");
- synchronized (lock)
- {
- spiReg = context.registerService(ConfigurationSpi.class.getName(), this,
+ spiReg = context.registerService(ConfigurationSpi.class.getName(), this,
props);
- }
- }
-
- synchronized (lock)
- {
- this.configs = Collections.unmodifiableMap(realmToConfigMap);
}
}
@@ -237,6 +234,12 @@ public class ConfigSpiOsgi extends Confi
void close()
{
+
+ if (spiReg != null)
+ {
+ spiReg.unregister();
+ }
+
this.tracker.close();
deregisterProvider(jaasConfigProviderName);
@@ -367,8 +370,24 @@ public class ConfigSpiOsgi extends Confi
public Object addingService(ServiceReference reference)
{
LoginModuleFactory lmf = (LoginModuleFactory) context.getService(reference);
- registerFactory(reference, lmf);
- recreateConfigs();
+ boolean registerSpi = false;
+ synchronized (lock)
+ {
+ boolean noConfigAtStart = configs.isEmpty();
+ registerFactory(reference, lmf);
+ recreateConfigs();
+ if (spiReg == null && noConfigAtStart && !configs.isEmpty())
+ {
+ registerSpi = true;
+ }
+ }
+
+ //Register SPI outside of this lock
+ if (registerSpi)
+ {
+ registerSpiWithOSGi();
+ }
+
return lmf;
}
@@ -380,14 +399,20 @@ public class ConfigSpiOsgi extends Confi
// refresh to update configs
((OsgiLoginModuleProvider) lmf).configure();
}
- recreateConfigs();
+ synchronized (lock)
+ {
+ recreateConfigs();
+ }
}
@Override
public void removedService(ServiceReference reference, Object service)
{
- deregisterFactory(reference);
- recreateConfigs();
+ synchronized (lock)
+ {
+ deregisterFactory(reference);
+ recreateConfigs();
+ }
context.ungetService(reference);
}
Modified: felix/trunk/jaas/src/test/java/org/apache/felix/jaas/internal/ITConcurrentLoginModuleFactoryTest.java
URL: http://svn.apache.org/viewvc/felix/trunk/jaas/src/test/java/org/apache/felix/jaas/internal/ITConcurrentLoginModuleFactoryTest.java?rev=1779601&r1=1779600&r2=1779601&view=diff
==============================================================================
--- felix/trunk/jaas/src/test/java/org/apache/felix/jaas/internal/ITConcurrentLoginModuleFactoryTest.java (original)
+++ felix/trunk/jaas/src/test/java/org/apache/felix/jaas/internal/ITConcurrentLoginModuleFactoryTest.java Fri Jan 20 10:36:52 2017
@@ -35,9 +35,10 @@ import java.util.Queue;
import java.util.concurrent.ArrayBlockingQueue;
import java.util.concurrent.CountDownLatch;
+import javax.security.auth.login.ConfigurationSpi;
+
import org.apache.felix.jaas.LoginModuleFactory;
import org.apache.sling.testing.mock.osgi.junit.OsgiContext;
-import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -47,7 +48,6 @@ import org.mockito.stubbing.Answer;
import org.osgi.framework.BundleContext;
import org.osgi.framework.ServiceReference;
-@Ignore("FELIX-5502")
@RunWith(Parameterized.class)
public class ITConcurrentLoginModuleFactoryTest
{
@@ -55,6 +55,7 @@ public class ITConcurrentLoginModuleFact
@Rule
public OsgiContext context = new OsgiContext();
+ //Run the test multiple times
@Parameterized.Parameters
public static List<Object[]> data()
{
@@ -97,6 +98,7 @@ public class ITConcurrentLoginModuleFact
assertEquals(numOfServices,
spi.engineGetAppConfigurationEntry(TEST_REALM_NAME).length);
+ assertEquals(1, context.getServices(ConfigurationSpi.class, null).length);
}
private static class ServiceAdder implements Runnable