You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by gn...@apache.org on 2014/03/31 18:13:09 UTC
svn commit: r1583364 - in
/felix/trunk/fileinstall/src/main/java/org/apache/felix/fileinstall/internal:
ConfigInstaller.java DirectoryWatcher.java FileInstall.java
Author: gnodet
Date: Mon Mar 31 16:13:09 2014
New Revision: 1583364
URL: http://svn.apache.org/r1583364
Log:
[FELIX-4472] Various concurrency issues in fileinstall
Modified:
felix/trunk/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/ConfigInstaller.java
felix/trunk/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/DirectoryWatcher.java
felix/trunk/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/FileInstall.java
Modified: felix/trunk/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/ConfigInstaller.java
URL: http://svn.apache.org/viewvc/felix/trunk/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/ConfigInstaller.java?rev=1583364&r1=1583363&r2=1583364&view=diff
==============================================================================
--- felix/trunk/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/ConfigInstaller.java (original)
+++ felix/trunk/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/ConfigInstaller.java Mon Mar 31 16:13:09 2014
@@ -51,24 +51,21 @@ public class ConfigInstaller implements
public void init()
{
- if (registration == null)
- {
- registration = this.context.registerService(
- new String[] {
- ConfigurationListener.class.getName(),
- ArtifactListener.class.getName(),
- ArtifactInstaller.class.getName()
- },
- this, null);
- }
+ registration = this.context.registerService(
+ new String[] {
+ ConfigurationListener.class.getName(),
+ ArtifactListener.class.getName(),
+ ArtifactInstaller.class.getName()
+ },
+ this, null);
}
public void destroy()
{
- if (registration != null)
- {
+ try {
registration.unregister();
- registration = null;
+ } catch (IllegalStateException e) {
+ // Ignore
}
}
Modified: felix/trunk/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/DirectoryWatcher.java
URL: http://svn.apache.org/viewvc/felix/trunk/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/DirectoryWatcher.java?rev=1583364&r1=1583363&r2=1583364&view=diff
==============================================================================
--- felix/trunk/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/DirectoryWatcher.java (original)
+++ felix/trunk/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/DirectoryWatcher.java Mon Mar 31 16:13:09 2014
@@ -254,79 +254,67 @@ public class DirectoryWatcher extends Th
{
// We must wait for FileInstall to complete initialisation
// to avoid race conditions observed in FELIX-2791
- synchronized (fileInstall.barrier)
+ try
{
- while (!fileInstall.initialized)
- {
- try
- {
- fileInstall.barrier.wait(0);
- }
- catch (InterruptedException e)
- {
- Thread.currentThread().interrupt();
- log(Logger.LOG_INFO, "Watcher for " + watchedDirectory + " exiting because of interruption.", e);
+ fileInstall.lock.readLock().lockInterruptibly();
+ }
+ catch (InterruptedException e)
+ {
+ Thread.currentThread().interrupt();
+ log(Logger.LOG_INFO, "Watcher for " + watchedDirectory + " exiting because of interruption.", e);
+ return;
+ }
+ try {
+ log(Logger.LOG_DEBUG,
+ "{" + POLL + " (ms) = " + poll + ", "
+ + DIR + " = " + watchedDirectory.getAbsolutePath() + ", "
+ + LOG_LEVEL + " = " + logLevel + ", "
+ + START_NEW_BUNDLES + " = " + startBundles + ", "
+ + TMPDIR + " = " + tmpDir + ", "
+ + FILTER + " = " + filter + ", "
+ + START_LEVEL + " = " + startLevel + "}", null
+ );
+
+ if (!noInitialDelay) {
+ try {
+ // enforce a delay before the first directory scan
+ Thread.sleep(poll);
+ } catch (InterruptedException e) {
+ log(Logger.LOG_DEBUG, "Watcher for " + watchedDirectory + " was interrupted while waiting "
+ + poll + " milliseconds for initial directory scan.", e);
return;
}
+ initializeCurrentManagedBundles();
}
}
- log(Logger.LOG_DEBUG,
- "{" + POLL + " (ms) = " + poll + ", "
- + DIR + " = " + watchedDirectory.getAbsolutePath() + ", "
- + LOG_LEVEL + " = " + logLevel + ", "
- + START_NEW_BUNDLES + " = " + startBundles + ", "
- + TMPDIR + " = " + tmpDir + ", "
- + FILTER + " = " + filter + ", "
- + START_LEVEL + " = " + startLevel + "}", null);
-
- if (!noInitialDelay)
+ finally
{
- try {
- // enforce a delay before the first directory scan
- Thread.sleep(poll);
- } catch (InterruptedException e) {
- log(Logger.LOG_DEBUG, "Watcher for " + watchedDirectory + " was interrupted while waiting "
- + poll + " milliseconds for initial directory scan.", e);
- return;
- }
- initializeCurrentManagedBundles();
+ fileInstall.lock.readLock().unlock();
}
- while (!interrupted())
- {
- try
- {
+ while (!interrupted()) {
+ try {
FrameworkStartLevel startLevelSvc = context.getBundle(0).adapt(FrameworkStartLevel.class);
// Don't access the disk when the framework is still in a startup phase.
if (startLevelSvc.getStartLevel() >= activeLevel
- && context.getBundle(0).getState() == Bundle.ACTIVE)
- {
+ && context.getBundle(0).getState() == Bundle.ACTIVE) {
Set<File> files = scanner.scan(false);
// Check that there is a result. If not, this means that the directory can not be listed,
// so it's presumably not a valid directory (it may have been deleted by someone).
// In such case, just sleep
- if (files != null)
- {
+ if (files != null) {
process(files);
}
}
- synchronized (this)
- {
+ synchronized (this) {
wait(poll);
}
- }
- catch (InterruptedException e)
- {
+ } catch (InterruptedException e) {
return;
- }
- catch (Throwable e)
- {
- try
- {
+ } catch (Throwable e) {
+ try {
context.getBundle();
- }
- catch (IllegalStateException t)
- {
+ } catch (IllegalStateException t) {
// FileInstall bundle has been uninstalled, exiting loop
return;
}
@@ -360,6 +348,19 @@ public class DirectoryWatcher extends Th
private void process(Set<File> files) throws InterruptedException
{
+ fileInstall.lock.readLock().lockInterruptibly();
+ try
+ {
+ doProcess(files);
+ }
+ finally
+ {
+ fileInstall.lock.readLock().unlock();
+ }
+ }
+
+ private void doProcess(Set<File> files) throws InterruptedException
+ {
List<ArtifactListener> listeners = fileInstall.getListeners();
List<Artifact> deleted = new ArrayList<Artifact>();
List<Artifact> modified = new ArrayList<Artifact>();
Modified: felix/trunk/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/FileInstall.java
URL: http://svn.apache.org/viewvc/felix/trunk/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/FileInstall.java?rev=1583364&r1=1583363&r2=1583364&view=diff
==============================================================================
--- felix/trunk/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/FileInstall.java (original)
+++ felix/trunk/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/FileInstall.java Mon Mar 31 16:13:09 2014
@@ -21,6 +21,8 @@ package org.apache.felix.fileinstall.int
import java.io.File;
import java.util.*;
import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
import org.apache.felix.fileinstall.ArtifactInstaller;
import org.apache.felix.fileinstall.ArtifactListener;
@@ -58,74 +60,77 @@ public class FileInstall implements Bund
BundleContext context;
final Map<String, DirectoryWatcher> watchers = new HashMap<String, DirectoryWatcher>();
ServiceTracker listenersTracker;
- boolean initialized;
- final Object barrier = new Object();
+ final ReadWriteLock lock = new ReentrantReadWriteLock();
ServiceRegistration urlHandlerRegistration;
public void start(BundleContext context) throws Exception
{
this.context = context;
-
- Hashtable<String, Object> props = new Hashtable<String, Object>();
- props.put("url.handler.protocol", JarDirUrlHandler.PROTOCOL);
- urlHandlerRegistration = context.registerService(org.osgi.service.url.URLStreamHandlerService.class.getName(), new JarDirUrlHandler(), props);
-
- String flt = "(|(" + Constants.OBJECTCLASS + "=" + ArtifactInstaller.class.getName() + ")"
- + "(" + Constants.OBJECTCLASS + "=" + ArtifactTransformer.class.getName() + ")"
- + "(" + Constants.OBJECTCLASS + "=" + ArtifactUrlTransformer.class.getName() + "))";
- listenersTracker = new ServiceTracker(context, FrameworkUtil.createFilter(flt), this);
- listenersTracker.open();
+ lock.writeLock().lock();
try
{
- cmSupport = new ConfigAdminSupport(context, this);
- }
- catch (NoClassDefFoundError e)
- {
- Util.log(context, Util.getGlobalLogLevel(context), Logger.LOG_DEBUG,
- "ConfigAdmin is not available, some features will be disabled", e);
- }
-
- // Created the initial configuration
- Hashtable<String, String> ht = new Hashtable<String, String>();
+ Hashtable<String, Object> props = new Hashtable<String, Object>();
+ props.put("url.handler.protocol", JarDirUrlHandler.PROTOCOL);
+ urlHandlerRegistration = context.registerService(org.osgi.service.url.URLStreamHandlerService.class.getName(), new JarDirUrlHandler(), props);
- set(ht, DirectoryWatcher.POLL);
- set(ht, DirectoryWatcher.DIR);
- set(ht, DirectoryWatcher.LOG_LEVEL);
- set(ht, DirectoryWatcher.FILTER);
- set(ht, DirectoryWatcher.TMPDIR);
- set(ht, DirectoryWatcher.START_NEW_BUNDLES);
- set(ht, DirectoryWatcher.USE_START_TRANSIENT);
- set(ht, DirectoryWatcher.NO_INITIAL_DELAY);
- set(ht, DirectoryWatcher.START_LEVEL);
+ String flt = "(|(" + Constants.OBJECTCLASS + "=" + ArtifactInstaller.class.getName() + ")"
+ + "(" + Constants.OBJECTCLASS + "=" + ArtifactTransformer.class.getName() + ")"
+ + "(" + Constants.OBJECTCLASS + "=" + ArtifactUrlTransformer.class.getName() + "))";
+ listenersTracker = new ServiceTracker(context, FrameworkUtil.createFilter(flt), this);
+ listenersTracker.open();
- // check if dir is an array of dirs
- String dirs = ht.get(DirectoryWatcher.DIR);
- if ( dirs != null && dirs.indexOf(',') != -1 )
- {
- StringTokenizer st = new StringTokenizer(dirs, ",");
- int index = 0;
- while ( st.hasMoreTokens() )
+ try
+ {
+ cmSupport = new ConfigAdminSupport(context, this);
+ }
+ catch (NoClassDefFoundError e)
{
- final String dir = st.nextToken().trim();
- ht.put(DirectoryWatcher.DIR, dir);
+ Util.log(context, Util.getGlobalLogLevel(context), Logger.LOG_DEBUG,
+ "ConfigAdmin is not available, some features will be disabled", e);
+ }
+
+ // Created the initial configuration
+ Hashtable<String, String> ht = new Hashtable<String, String>();
+
+ set(ht, DirectoryWatcher.POLL);
+ set(ht, DirectoryWatcher.DIR);
+ set(ht, DirectoryWatcher.LOG_LEVEL);
+ set(ht, DirectoryWatcher.FILTER);
+ set(ht, DirectoryWatcher.TMPDIR);
+ set(ht, DirectoryWatcher.START_NEW_BUNDLES);
+ set(ht, DirectoryWatcher.USE_START_TRANSIENT);
+ set(ht, DirectoryWatcher.NO_INITIAL_DELAY);
+ set(ht, DirectoryWatcher.START_LEVEL);
+
+ // check if dir is an array of dirs
+ String dirs = ht.get(DirectoryWatcher.DIR);
+ if (dirs != null && dirs.indexOf(',') != -1)
+ {
+ StringTokenizer st = new StringTokenizer(dirs, ",");
+ int index = 0;
+ while (st.hasMoreTokens())
+ {
+ final String dir = st.nextToken().trim();
+ ht.put(DirectoryWatcher.DIR, dir);
- String name = "initial";
- if ( index > 0 ) name = name + index;
- updated(name, new Hashtable<String, String>(ht));
+ String name = "initial";
+ if (index > 0) name = name + index;
+ updated(name, new Hashtable<String, String>(ht));
- index++;
+ index++;
+ }
+ }
+ else
+ {
+ updated("initial", ht);
}
}
- else
+ finally
{
- updated("initial", ht);
- }
- // now notify all the directory watchers to proceed
- // We need this to avoid race conditions observed in FELIX-2791
- synchronized (barrier) {
- initialized = true;
- barrier.notifyAll();
+ // now notify all the directory watchers to proceed
+ // We need this to avoid race conditions observed in FELIX-2791
+ lock.writeLock().unlock();
}
}
@@ -162,34 +167,39 @@ public class FileInstall implements Bund
public void stop(BundleContext context) throws Exception
{
- synchronized (barrier) {
- initialized = false;
- }
- urlHandlerRegistration.unregister();
- List<DirectoryWatcher> toClose = new ArrayList<DirectoryWatcher>();
- synchronized (watchers)
- {
- toClose.addAll(watchers.values());
- watchers.clear();
- }
- for (DirectoryWatcher aToClose : toClose)
+ lock.writeLock().lock();
+ try
{
- try
+ urlHandlerRegistration.unregister();
+ List<DirectoryWatcher> toClose = new ArrayList<DirectoryWatcher>();
+ synchronized (watchers)
{
- aToClose.close();
+ toClose.addAll(watchers.values());
+ watchers.clear();
}
- catch (Exception e)
+ for (DirectoryWatcher aToClose : toClose)
{
- // Ignore
+ try
+ {
+ aToClose.close();
+ }
+ catch (Exception e)
+ {
+ // Ignore
+ }
+ }
+ if (listenersTracker != null)
+ {
+ listenersTracker.close();
+ }
+ if (cmSupport != null)
+ {
+ cmSupport.run();
}
}
- if (listenersTracker != null)
- {
- listenersTracker.close();
- }
- if (cmSupport != null)
+ finally
{
- cmSupport.run();
+ lock.writeLock().unlock();
}
}
@@ -307,7 +317,7 @@ public class FileInstall implements Bund
latch.await();
}
- private static class ConfigAdminSupport implements Runnable
+ private class ConfigAdminSupport implements Runnable
{
private Tracker tracker;
private ServiceRegistration registration;
@@ -323,15 +333,15 @@ public class FileInstall implements Bund
public void run()
{
- registration.unregister();
tracker.close();
+ registration.unregister();
}
private class Tracker extends ServiceTracker<ConfigurationAdmin, ConfigurationAdmin> implements ManagedServiceFactory {
private final FileInstall fileInstall;
private final Set<String> configs = Collections.synchronizedSet(new HashSet<String>());
- private ConfigInstaller configInstaller;
+ private final Map<Long, ConfigInstaller> configInstallers = new HashMap<Long, ConfigInstaller>();
private Tracker(BundleContext bundleContext, FileInstall fileInstall)
{
@@ -363,26 +373,46 @@ public class FileInstall implements Bund
public ConfigurationAdmin addingService(ServiceReference<ConfigurationAdmin> serviceReference)
{
- ConfigurationAdmin cm = super.addingService(serviceReference);
- configInstaller = new ConfigInstaller(this.context, cm, fileInstall);
- configInstaller.init();
- return cm;
+ lock.writeLock().lock();
+ try
+ {
+ ConfigurationAdmin cm = super.addingService(serviceReference);
+ long id = (Long) serviceReference.getProperty(Constants.SERVICE_ID);
+ ConfigInstaller configInstaller = new ConfigInstaller(this.context, cm, fileInstall);
+ configInstaller.init();
+ configInstallers.put(id, configInstaller);
+ return cm;
+ }
+ finally
+ {
+ lock.writeLock().unlock();
+ }
}
public void removedService(ServiceReference<ConfigurationAdmin> serviceReference, ConfigurationAdmin o)
{
- Iterator iterator = configs.iterator();
- while (iterator.hasNext()) {
- String s = (String) iterator.next();
- fileInstall.deleted(s);
- iterator.remove();
+ lock.writeLock().lock();
+ try
+ {
+ Iterator iterator = configs.iterator();
+ while (iterator.hasNext())
+ {
+ String s = (String) iterator.next();
+ fileInstall.deleted(s);
+ iterator.remove();
+ }
+ long id = (Long) serviceReference.getProperty(Constants.SERVICE_ID);
+ ConfigInstaller configInstaller = configInstallers.remove(id);
+ if (configInstaller != null)
+ {
+ configInstaller.destroy();
+ }
+ super.removedService(serviceReference, o);
}
- if (configInstaller != null)
+ finally
{
- configInstaller.destroy();
- configInstaller = null;
+ lock.writeLock().unlock();
}
- super.removedService(serviceReference, o);
}
}
}