Author: djencks
Date: Mon Mar 4 08:28:05 2013
New Revision: 1452208
URL: http://svn.apache.org/r1452208
Log:
FELIX-3891 Fix the new way to avoid deadlocks, and preserve state change
ordering, with an actual unit test
Added:
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/RegistrationManager.java
felix/trunk/scr/src/test/java/org/apache/felix/scr/impl/manager/RegistrationManagerTest.java
Modified:
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java
Modified:
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java
URL:
http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java?rev=1452208&r1=1452207&r2=1452208&view=diff
==============================================================================
---
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java
(original)
+++
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java
Mon Mar 4 08:28:05 2013
@@ -97,14 +97,8 @@ public abstract class AbstractComponentM
// A reference to the BundleComponentActivator
private BundleComponentActivator m_activator;
- // The ServiceRegistration
- private enum RegState {unregistered, registered};
- private final Lock registrationLock = new ReentrantLock();
- //Deque, ArrayDeque if we had java 6
- private final List<RegState> opqueue = new ArrayList<RegState>();
-
- private volatile ServiceRegistration<S> m_serviceRegistration;
-
+ // The ServiceRegistration is now tracked in the RegistrationManager
+
private final ReentrantLock m_stateLock;
protected volatile boolean enabled;
@@ -652,7 +646,7 @@ public abstract class AbstractComponentM
final ServiceReference getServiceReference()
{
- ServiceRegistration reg = m_serviceRegistration;
+ ServiceRegistration reg = getServiceRegistration();
if (reg != null)
{
return reg.getReference();
@@ -702,6 +696,34 @@ public abstract class AbstractComponentM
return null;
}
+
+ private final RegistrationManager<ServiceRegistration<S>>
registrationManager = new RegistrationManager<ServiceRegistration<S>>()
+ {
+
+ @Override
+ ServiceRegistration<S> register(String[] services)
+ {
+ final Dictionary<String, Object> serviceProperties =
getServiceProperties();
+ ServiceRegistration<S> serviceRegistration = (
ServiceRegistration<S> ) getActivator().getBundleContext()
+ .registerService( services, getService(),
serviceProperties );
+ return serviceRegistration;
+ }
+
+ @Override
+ void unregister(ServiceRegistration<S> serviceRegistration)
+ {
+ serviceRegistration.unregister();
+ }
+
+ @Override
+ void log(int level, String message, Object[] arguments, Throwable ex)
+ {
+ AbstractComponentManager.this.log(level, message, arguments, ex);
+ }
+
+ };
+
+
/**
* Registers the service on behalf of the component.
*
@@ -711,7 +733,7 @@ public abstract class AbstractComponentM
String[] services = getProvidedServices();
if ( services != null )
{
- return changeRegistration( RegState.registered, services);
+ return registrationManager.changeRegistration(
RegistrationManager.RegState.registered, services);
}
return true;
}
@@ -721,92 +743,11 @@ public abstract class AbstractComponentM
String[] services = getProvidedServices();
if ( services != null )
{
- return changeRegistration( RegState.unregistered, services );
+ return registrationManager.changeRegistration(
RegistrationManager.RegState.unregistered, services );
}
return true;
}
- /**
- *
- * @param desired
- * @return true if this thread reached the requested state
- */
- private boolean changeRegistration( RegState desired, String[] services )
- {
- registrationLock.lock();
- try
- {
- if (opqueue.isEmpty())
- {
- if ((desired == RegState.unregistered) ==
(m_serviceRegistration == null))
- {
- return false; //already in desired state
- }
- }
- else if (opqueue.get(0) == desired)
- {
- return false; //another thread will do our work.
- }
- opqueue.add( desired );
- if (opqueue.size() > 1)
- {
- return false; //some other thread will do it later
- }
- //we're next
- do
- {
- log( LogService.LOG_DEBUG, "registration change queue {0}",
new Object[]
- {opqueue}, null );
- desired = opqueue.get( 0 );
- ServiceRegistration<S> serviceRegistration =
m_serviceRegistration;
- if ( desired == RegState.unregistered)
- {
- m_serviceRegistration = null;
- }
-
- registrationLock.unlock();
- try
- {
- if (desired == RegState.registered)
- {
- final Dictionary<String, Object> serviceProperties =
getServiceProperties();
- serviceRegistration = ( ServiceRegistration<S> )
getActivator().getBundleContext()
- .registerService( services, getService(),
serviceProperties );
-
- }
- else
- {
- if ( serviceRegistration != null )
- {
- serviceRegistration.unregister();
- }
- else
- {
- log( LogService.LOG_ERROR, "Unexpected
unregistration request with no registration present", new Object[]
- {}, new Exception("Stack trace") );
-
- }
- }
- }
- finally
- {
- registrationLock.lock();
- opqueue.remove(0);
- if ( desired == RegState.registered)
- {
- m_serviceRegistration = serviceRegistration;
- }
- }
- }
- while (!opqueue.isEmpty());
- return true;
- }
- finally
- {
- registrationLock.unlock();
- }
-
- }
AtomicInteger getTrackingCount()
{
@@ -907,9 +848,9 @@ public abstract class AbstractComponentM
}
- final ServiceRegistration<?> getServiceRegistration()
+ final ServiceRegistration<S> getServiceRegistration()
{
- return m_serviceRegistration;
+ return registrationManager.getServiceRegistration();
}
@@ -1225,7 +1166,7 @@ public abstract class AbstractComponentM
void changeState( State newState )
{
log( LogService.LOG_DEBUG, "State transition : {0} -> {1} : service
reg: {2}", new Object[]
- { m_state, newState, m_serviceRegistration }, null );
+ { m_state, newState, getServiceRegistration() }, null );
m_state = newState;
}
@@ -1330,7 +1271,7 @@ public abstract class AbstractComponentM
private void log( AbstractComponentManager acm, String event )
{
acm.log( LogService.LOG_DEBUG, "Current state: {0}, Event: {1},
Service registration: {2}", new Object[]
- { m_name, event, acm.m_serviceRegistration }, null );
+ { m_name, event, acm.getServiceRegistration() }, null );
}
void doDeactivate( AbstractComponentManager acm, int reason, boolean
disable )
Added:
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/RegistrationManager.java
URL:
http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/RegistrationManager.java?rev=1452208&view=auto
==============================================================================
---
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/RegistrationManager.java
(added)
+++
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/RegistrationManager.java
Mon Mar 4 08:28:05 2013
@@ -0,0 +1,111 @@
+package org.apache.felix.scr.impl.manager;
+
+import java.util.ArrayList;
+import java.util.Dictionary;
+import java.util.List;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+import org.osgi.service.log.LogService;
+
+abstract class RegistrationManager<T>
+{
+ enum RegState {unregistered, registered};
+ private final Lock registrationLock = new ReentrantLock();
+ //Deque, ArrayDeque if we had java 6
+ private final List<RegState> opqueue = new ArrayList<RegState>();
+
+ private volatile T m_serviceRegistration;
+ /**
+ *
+ * @param desired
+ * @param services TODO
+ * @return true if this thread reached the requested state
+ */
+ boolean changeRegistration( RegState desired, String[] services )
+ {
+ registrationLock.lock();
+ try
+ {
+ if (opqueue.isEmpty())
+ {
+ if ((desired == RegState.unregistered) ==
(m_serviceRegistration == null))
+ {
+ return false; //already in desired state
+ }
+ }
+ else if (opqueue.get( opqueue.size() - 1 ) == desired)
+ {
+ return false; //another thread will do our work.
+ }
+ opqueue.add( desired );
+ if (opqueue.size() > 1)
+ {
+ return false; //some other thread will do it later
+ }
+ //we're next
+ do
+ {
+ log( LogService.LOG_DEBUG, "registration change queue {0}",
new Object[]
+ {opqueue}, null );
+ desired = opqueue.get( 0 );
+ T serviceRegistration = m_serviceRegistration;
+ if ( desired == RegState.unregistered)
+ {
+ m_serviceRegistration = null;
+ }
+
+ registrationLock.unlock();
+ try
+ {
+ if (desired == RegState.registered)
+ {
+ serviceRegistration = register(services );
+
+ }
+ else
+ {
+ if ( serviceRegistration != null )
+ {
+ unregister( serviceRegistration );
+ }
+ else
+ {
+ log( LogService.LOG_ERROR, "Unexpected
unregistration request with no registration present", new Object[]
+ {}, new Exception("Stack trace") );
+
+ }
+ }
+ }
+ finally
+ {
+ registrationLock.lock();
+ opqueue.remove(0);
+ if ( desired == RegState.registered)
+ {
+ m_serviceRegistration = serviceRegistration;
+ }
+ }
+ }
+ while (!opqueue.isEmpty());
+ return true;
+ }
+ finally
+ {
+ registrationLock.unlock();
+ }
+
+ }
+
+ abstract T register(String[] services);
+
+ abstract void unregister(T serviceRegistration);
+
+ abstract void log( int level, String message, Object[] arguments,
Throwable ex );
+
+ T getServiceRegistration()
+ {
+ return m_serviceRegistration;
+ }
+
+}
Added:
felix/trunk/scr/src/test/java/org/apache/felix/scr/impl/manager/RegistrationManagerTest.java
URL:
http://svn.apache.org/viewvc/felix/trunk/scr/src/test/java/org/apache/felix/scr/impl/manager/RegistrationManagerTest.java?rev=1452208&view=auto
==============================================================================
---
felix/trunk/scr/src/test/java/org/apache/felix/scr/impl/manager/RegistrationManagerTest.java
(added)
+++
felix/trunk/scr/src/test/java/org/apache/felix/scr/impl/manager/RegistrationManagerTest.java
Mon Mar 4 08:28:05 2013
@@ -0,0 +1,113 @@
+package org.apache.felix.scr.impl.manager;
+
+import static org.junit.Assert.fail;
+
+import java.util.ArrayList;
+import java.util.concurrent.CountDownLatch;
+
+import org.apache.felix.scr.impl.manager.RegistrationManager.RegState;
+import org.junit.Test;
+
+
+public class RegistrationManagerTest
+{
+
+ private volatile boolean fail;
+
+ private int n = 10;
+ private ArrayList<Thread> threads = new ArrayList<Thread>();
+
+ private TRM trm = new TRM();
+
+ @Test
+ public void testRegistrationManager( ) throws Exception
+ {
+
+ for (int setup = 0; setup < 1 << n; setup++ )
+ {
+ runTest(setup);
+ if (fail)
+ {
+ fail("failed at " + setup);
+ }
+ }
+ }
+
+ private void runTest(int setup) throws InterruptedException
+ {
+ final CountDownLatch done = new CountDownLatch( n );
+ for (int i = 0; i < n; i++)
+ {
+ boolean b = ((setup >>> i) & 1) == 0;
+ final RegState change = b? RegState.registered:
RegState.unregistered;
+ new Thread(new Runnable() {
+
+ public void run()
+ {
+ trm.changeRegistration( change, null );
+ }
+
+ }).start();
+ done.countDown();
+ }
+ done.await();
+ }
+
+
+ private class TRM extends RegistrationManager<Object>
+ {
+
+ @Override
+ Object register(String[] services)
+ {
+ try
+ {
+ Thread.sleep( 1 );
+ }
+ catch ( InterruptedException e )
+ {
+ // TODO Auto-generated catch block
+ e.printStackTrace();
+ }
+ return new Object();
+ }
+
+ @Override
+ void unregister(Object serviceRegistration)
+ {
+ try
+ {
+ Thread.sleep( 1 );
+ }
+ catch ( InterruptedException e )
+ {
+ // TODO Auto-generated catch block
+ e.printStackTrace();
+ }
+ }
+
+ @Override
+ void log(int level, String message, Object[] arguments, Throwable ex)
+ {
+ if ( arguments.length == 1 && (arguments[0] instanceof ArrayList))
+ {
+ ArrayList<RegState> opqueue = (
ArrayList<org.apache.felix.scr.impl.manager.RegistrationManager.RegState> )
arguments[0];
+// System.out.println("opqueue: " + opqueue);
+ if (opqueue.size() > 1)
+ {
+ for (int i = 1; i < opqueue.size(); i++)
+ {
+ if (opqueue.get( i -1 ) == opqueue.get(i))
+ {
+ fail = true;
+ System.out.println("opqueue: " + opqueue);
+ return;
+ }
+ }
+ }
+ }
+
+ }
+
+ }
+}
