felix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pde...@apache.org
Subject svn commit: r1776903 - in /felix/trunk/dependencymanager: org.apache.felix.dependencymanager.itest/src/org/apache/felix/dm/itest/api/ org.apache.felix.dependencymanager/src/org/apache/felix/dm/ org.apache.felix.dependencymanager/src/org/apache/felix/dm...
Date Mon, 02 Jan 2017 08:20:08 GMT
Author: pderop
Date: Mon Jan  2 08:20:08 2017
New Revision: 1776903

URL: http://svn.apache.org/viewvc?rev=1776903&view=rev
Log:
FELIX-5471: Removed the timeout guard blocking code in ComponentImpl.schedule method because
it may produces a deadlock (see FELIX5471_CyclicDependencyTest.java).

Added:
    felix/trunk/dependencymanager/org.apache.felix.dependencymanager.itest/src/org/apache/felix/dm/itest/api/FELIX5471_CyclicDependencyTest.java
Modified:
    felix/trunk/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/DependencyManager.java
    felix/trunk/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/ComponentImpl.java
    felix/trunk/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/InvocationUtil.java

Added: felix/trunk/dependencymanager/org.apache.felix.dependencymanager.itest/src/org/apache/felix/dm/itest/api/FELIX5471_CyclicDependencyTest.java
URL: http://svn.apache.org/viewvc/felix/trunk/dependencymanager/org.apache.felix.dependencymanager.itest/src/org/apache/felix/dm/itest/api/FELIX5471_CyclicDependencyTest.java?rev=1776903&view=auto
==============================================================================
--- felix/trunk/dependencymanager/org.apache.felix.dependencymanager.itest/src/org/apache/felix/dm/itest/api/FELIX5471_CyclicDependencyTest.java
(added)
+++ felix/trunk/dependencymanager/org.apache.felix.dependencymanager.itest/src/org/apache/felix/dm/itest/api/FELIX5471_CyclicDependencyTest.java
Mon Jan  2 08:20:08 2017
@@ -0,0 +1,108 @@
+/*
+ * 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.felix.dm.itest.api;
+
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+import org.apache.felix.dm.Component;
+import org.apache.felix.dm.ComponentState;
+import org.apache.felix.dm.ComponentStateListener;
+import org.apache.felix.dm.DependencyManager;
+import org.apache.felix.dm.itest.util.Ensure;
+import org.apache.felix.dm.itest.util.TestBase;
+
+/**
+ * Verifies if a concurrent deactivation of two components depending on each other does not
produce a deadlock.
+ */
+public class FELIX5471_CyclicDependencyTest extends TestBase {
+
+	volatile Ensure m_ensure;
+
+	public void cyclicDependencyTest() throws InterruptedException {
+		DependencyManager m = getDM();
+		ExecutorService tpool = Executors.newFixedThreadPool(2);
+		try {
+			for (int count = 0; count < 1000; count++) {
+				m_ensure = new Ensure();
+
+				Component a = m.createComponent()
+						.setImplementation(new A())
+						.setInterface(A.class.getName(), null)
+						.add(m.createServiceDependency().setService(B.class).setRequired(true).setCallbacks("add",
"remove"));
+
+				Component b = m.createComponent()
+						.setImplementation(new B())
+						.setInterface(B.class.getName(), null)
+						.add(m.createServiceDependency().setService(A.class).setRequired(false).setCallbacks("add",
"remove"));
+				
+				m.add(a);
+				m.add(b);			
+
+				ComponentStateListener l = (c, s) -> {
+					if (s == ComponentState.INACTIVE) {
+						m_ensure.step();
+					}
+				};
+				a.add(l);
+				b.add(l);
+				
+				m_ensure.waitForStep(4, 50000); // A started, B started
+
+				tpool.execute(() -> m.remove(a));
+				tpool.execute(() -> m.remove(b));
+				
+				m_ensure.waitForStep(10, 50000); // A unbound from  B, stopped and inactive, B unbound
from A, stopped and inactive
+			}
+		} finally {
+			tpool.shutdown();
+		}
+	}
+
+	public class Base {
+		void start() {
+			m_ensure.step();
+		}
+
+		void stop() {
+			m_ensure.step();
+		}
+
+	}
+
+	public class A extends Base {
+		public void add(B b) {
+			m_ensure.step();
+		}
+
+		public void remove(B b) {
+			m_ensure.step();
+		}
+	}
+
+	public class B extends Base {
+		public void add(A a) {
+			m_ensure.step();
+		}
+
+		public void remove(A a) {
+			m_ensure.step();
+		}
+	}
+}

Modified: felix/trunk/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/DependencyManager.java
URL: http://svn.apache.org/viewvc/felix/trunk/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/DependencyManager.java?rev=1776903&r1=1776902&r2=1776903&view=diff
==============================================================================
--- felix/trunk/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/DependencyManager.java
(original)
+++ felix/trunk/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/DependencyManager.java
Mon Jan  2 08:20:08 2017
@@ -70,19 +70,6 @@ public class DependencyManager {
     public static final String SERVICEREGISTRY_CACHE_INDICES = "org.apache.felix.dependencymanager.filterindex";
     public static final String METHOD_CACHE_SIZE = "org.apache.felix.dependencymanager.methodcache";
     
-    /**
-     * Max time we can wait for execution of some tasks which must be handled synchronously
through the internal component
-     * executor (which can be a threadpool).
-     * Typically, this timeout is used when a component is stopped or if a dependency is
being unbound from a given
-     * component.
-     */
-    public static final String SCHEDULE_TIMEOUT = "org.apache.felix.dependencymanager.scheduletimeout";
-    
-    /**
-     * Default value for the SCHEDULE_TIMEOUT parameter, in millis.
-     */
-    public static volatile long SCHEDULE_TIMEOUT_VAL = 30000;
-
     private final BundleContext m_context;
     private final Logger m_logger;
     private final ConcurrentHashMap<Component, Component> m_components = new ConcurrentHashMap<>();
@@ -115,14 +102,6 @@ public class DependencyManager {
 	            		}
 	            	}
 	            }
-	            
-	            String scheduleTimeout = bundleContext.getProperty(SCHEDULE_TIMEOUT);
-	            if (scheduleTimeout != null) {
-	            	try {
-	            		SCHEDULE_TIMEOUT_VAL = Long.valueOf(scheduleTimeout);
-	            	} catch (NumberFormatException e) {	            		
-	            	}
-	            }	            
 	        }
         }
         catch (BundleException e) {

Modified: felix/trunk/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/ComponentImpl.java
URL: http://svn.apache.org/viewvc/felix/trunk/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/ComponentImpl.java?rev=1776903&r1=1776902&r2=1776903&view=diff
==============================================================================
--- felix/trunk/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/ComponentImpl.java
(original)
+++ felix/trunk/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/ComponentImpl.java
Mon Jan  2 08:20:08 2017
@@ -43,11 +43,7 @@ import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentSkipListSet;
 import java.util.concurrent.CopyOnWriteArrayList;
-import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Executor;
-import java.util.concurrent.FutureTask;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicLong;
 
@@ -425,19 +421,10 @@ public class ComponentImpl implements Co
 	@Override
 	public void stop() {           
 	    if (m_active.compareAndSet(true, false)) {
-	    	Runnable task = () -> {
-	            m_isStarted = false;
+	    	schedule(true /* try execute synchronously if using a tpool */, () -> { 
+	    		m_isStarted = false;
 	            handleChange();
-	    	};
-	    	
-    		Executor exec = getExecutor();
-    		if (exec instanceof DispatchExecutor) {
-                // Now, we have to schedule our stopTask in our component executor. If the
executor is a parallel 
-    	        // dispatcher, then try to invoke our stop task synchronously (it does not make
sense to try to stop a component asynchronously).
-    			((DispatchExecutor) exec).execute(task, false);
-    		} else {
-    			exec.execute(task);
-    		}
+	    	});
 	    }
 	}
 
@@ -463,33 +450,36 @@ public class ComponentImpl implements Co
     public void handleEvent(final DependencyContext dc, final EventType type, final Event...
event) {
         // since this method can be invoked by anyone from any thread, we need to
         // pass on the event to a runnable that we execute using the component's
-        // executor. There is one corner case: if this is a REMOVE event, we must try to
stay synchronous
-		// because if the remove event corresponds to a service being unregistered, then we must
try to stop 
-		// our component depending on the lost service before the lost service is actually stopped.
	
-		boolean synchronously = (type == EventType.REMOVED);
+        // executor. There is one corner case: if this is a REMOVE event, and if we are using
a threadpool,
+		// then make a best effort to try invoking the component unbind callback synchronously
(to make 
+		// sure the unbound service is not stopped at the time we call unbind on our component
+		// which depends on the removing service).
+		// This is just a best effort, and the removed event will be handled asynchronosly if our

+		// queue is currently being run by another thread, or by the threadpool.
 		
-		schedule(synchronously, () -> {
-                try {
-                    switch (type) {
-                    case ADDED:
-                        handleAdded(dc, event[0]);
-                        break;
-                    case CHANGED:
-                        handleChanged(dc, event[0]);
-                        break;
-                    case REMOVED:
-                        handleRemoved(dc, event[0]);
-                        break;
-                    case SWAPPED:
-                        handleSwapped(dc, event[0], event[1]);
-                        break;
-                    }
-                } finally {
-                	// Clear cache of component callbacks invocation, except if we are currently
called from handleChange().
-                	// (See FELIX-4913).
-                    clearInvokeCallbackCache();
-                }
-            });        
+		boolean synchronously = (type == EventType.REMOVED);
+		schedule(synchronously, () ->  {
+			try {
+				switch (type) {
+				case ADDED:
+					handleAdded(dc, event[0]);
+					break;
+				case CHANGED:
+					handleChanged(dc, event[0]);
+					break;
+				case REMOVED:
+					handleRemoved(dc, event[0]);
+					break;
+				case SWAPPED:
+					handleSwapped(dc, event[0], event[1]);
+					break;
+				}
+			} finally {
+				// Clear cache of component callbacks invocation, except if we are currently called from
handleChange().
+				// (See FELIX-4913).
+				clearInvokeCallbackCache();
+			}
+		});
 	}
 
     @Override
@@ -1686,23 +1676,25 @@ public class ComponentImpl implements Co
         }
     }
     
-    private void schedule(boolean synchronously, Runnable task) {
-    	if (synchronously) {
-    		FutureTask<Void> future = new FutureTask<Void>(task, null);
-    		Executor exec = getExecutor();
+    /**
+     * Executes a task using our queue. The task is executed synchronously in case the queue
is 
+     * not being run by another thread, or by the threadpool.
+     * 
+     * @param trySynchronous try to execute the task synchronously (best effort).
+     * @param task the task to execute.
+     */
+    private void schedule(boolean trySynchronous, Runnable task) {
+		Executor exec = getExecutor();
+		if (trySynchronous) {
     		if (exec instanceof DispatchExecutor) {
-    			// try to invoke the future from the current thread, not using the threadpool.
-    			((DispatchExecutor) exec).execute(future, false);
-    		} else {
-    			exec.execute(future);
+    			// Try to execute the task from the current thread if the threadpool is not currently
running our queue.
+    			((DispatchExecutor) exec).execute(task, false);
+    			return;
     		}
-    		try {
-				future.get(DependencyManager.SCHEDULE_TIMEOUT_VAL, TimeUnit.MILLISECONDS);
-			} catch (InterruptedException | ExecutionException | TimeoutException e) {
-				m_logger.warn("task could not be scheduled timely in component %s (exception: %s)", this,
e.toString());
-			}
-    	} else {
-    		getExecutor().execute(task);
-    	}
-    }    
+		}
+		// If we are using a serial queue (no threadpool), then the queue executes the task synchronously

+		// if no other master thread is running the queue.
+		// If the are using a threadpool, then the task is always executed asynchronously, in the
threadpool.
+		exec.execute(task);
+    }
 }

Modified: felix/trunk/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/InvocationUtil.java
URL: http://svn.apache.org/viewvc/felix/trunk/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/InvocationUtil.java?rev=1776903&r1=1776902&r2=1776903&view=diff
==============================================================================
--- felix/trunk/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/InvocationUtil.java
(original)
+++ felix/trunk/dependencymanager/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/InvocationUtil.java
Mon Jan  2 08:20:08 2017
@@ -234,7 +234,7 @@ public class InvocationUtil {
         queue.execute(ft);
                 
         try {
-            Exception err = ft.get(DependencyManager.SCHEDULE_TIMEOUT_VAL, TimeUnit.MILLISECONDS);
+            Exception err = ft.get();
             if (err != null) {
                 throw err;
             }



Mime
View raw message