Re: svn commit: r1232143 - /karaf/trunk/features/core/src/main/java/org/apache/karaf/features/internal/FeaturesServiceImpl.java

[Guillaume Nodet <gn...@gmail.com>]

I just came across this bit of code which looks problematic to me.
The latch count down can only be used once, so if the region persistence
bundle is stopped for example, the getRegionsPersistence will never return.
Also the queue is not synchronized so that may cause issues ...
Why not using a simple optional reference in blueprint ? Given the code
just wait for a service to become available, that's exactly what the
blueprint proxies do.

On Mon, Jan 16, 2012 at 9:13 PM, <io...@apache.org> wrote:

> Author: iocanel
> Date: Mon Jan 16 20:13:49 2012
> New Revision: 1232143
>
> URL: http://svn.apache.org/viewvc?rev=1232143&view=rev
> Log:
> [KARAF-1136] FeaturesServiceImpl will wait for at most 5 seconds for the
> RegionsPersistence service to become available, if need.
>
> Modified:
>
> karaf/trunk/features/core/src/main/java/org/apache/karaf/features/internal/FeaturesServiceImpl.java
>
> Modified:
> karaf/trunk/features/core/src/main/java/org/apache/karaf/features/internal/FeaturesServiceImpl.java
> URL:
> http://svn.apache.org/viewvc/karaf/trunk/features/core/src/main/java/org/apache/karaf/features/internal/FeaturesServiceImpl.java?rev=1232143&r1=1232142&r2=1232143&view=diff
>
> ==============================================================================
> ---
> karaf/trunk/features/core/src/main/java/org/apache/karaf/features/internal/FeaturesServiceImpl.java
> (original)
> +++
> karaf/trunk/features/core/src/main/java/org/apache/karaf/features/internal/FeaturesServiceImpl.java
> Mon Jan 16 20:13:49 2012
> @@ -47,6 +47,8 @@ import java.util.Queue;
>  import java.util.Set;
>  import java.util.TreeSet;
>  import java.util.concurrent.CopyOnWriteArrayList;
> +import java.util.concurrent.CountDownLatch;
> +import java.util.concurrent.TimeUnit;
>  import java.util.concurrent.atomic.AtomicBoolean;
>  import java.util.jar.JarInputStream;
>  import java.util.jar.Manifest;
> @@ -113,10 +115,12 @@ public class FeaturesServiceImpl impleme
>      AtomicBoolean bootFeaturesInstalled = new AtomicBoolean();
>      private List<FeaturesListener> listeners = new
> CopyOnWriteArrayList<FeaturesListener>();
>      private Queue<RegionsPersistence> regionsPersistenceQueue = new
> LinkedList<RegionsPersistence>();
> +    private CountDownLatch regionsPersistenceLatch = new
> CountDownLatch(1);
>      private ThreadLocal<Repository> repo = new ThreadLocal<Repository>();
>      private EventAdminListener eventAdminListener;
>      private final Object refreshLock = new Object();
>      private long refreshTimeout = 5000;
> +    private long regionsPersistenceTimeout = 5000;
>
>      public FeaturesServiceImpl() {
>      }
> @@ -172,8 +176,27 @@ public class FeaturesServiceImpl impleme
>      }
>
>
> +    /**
> +     * Returns a RegionsPersistence service.
> +     * @param timeout
> +     * @return
> +     */
> +    protected RegionsPersistence getRegionsPersistence(long timeout) {
> +        if (!regionsPersistenceQueue.isEmpty()) {
> +            return regionsPersistenceQueue.peek();
> +        } else {
> +            try {
> +                regionsPersistenceLatch.await(timeout,
> TimeUnit.MILLISECONDS);
> +            } catch (InterruptedException e) {
> +                LOGGER.warn("Interrupted while waittng for
> RegionsPersistence service",e);
> +            }
> +            return regionsPersistenceQueue.peek();
> +        }
> +    }
> +
>      public void registerRegionsPersistence(RegionsPersistence
> regionsPersistence) {
>          regionsPersistenceQueue.add(regionsPersistence);
> +        regionsPersistenceLatch.countDown();
>      }
>
>      public void unregisterRegionsPersistence(RegionsPersistence
> regionsPersistence) {
> @@ -489,9 +512,13 @@ public class FeaturesServiceImpl impleme
>              Bundle b = installBundleIfNeeded(state, bInfo,
> feature.getStartLevel(), verbose);
>              bundles.add(b.getBundleId());
>              state.bundleInfos.put(b.getBundleId(), bInfo);
> -            RegionsPersistence regionsPersistence =
> regionsPersistenceQueue.peek();
> -            if (region != null && state.installed.contains(b) &&
> regionsPersistence != null) {
> -                regionsPersistence.install(b, region);
> +            if (region != null && state.installed.contains(b)) {
> +                RegionsPersistence regionsPersistence =
> getRegionsPersistence(regionsPersistenceTimeout);
> +                if (regionsPersistence != null) {
> +                    regionsPersistence.install(b, region);
> +                } else {
> +                    throw new Exception("Unable to find
> RegionsPersistence service, while installing "+ feature.getName());
> +                }
>              }
>          }
>          state.features.put(feature, bundles);
>
>
>


-- 
------------------------
Guillaume Nodet
------------------------
Blog: http://gnodet.blogspot.com/
------------------------
FuseSource, Integration everywhere
http://fusesource.com

Re: svn commit: r1232143 - /karaf/trunk/features/core/src/main/java/org/apache/karaf/features/internal/FeaturesServiceImpl.java

[Ioannis Canellos <io...@gmail.com>]

To be honest I don't quite remember the purpose of it.
I'll get on it asap.

-- 
*Ioannis Canellos*
*
FuseSource <http://fusesource.com>

**
Blog: http://iocanel.blogspot.com
**
Twitter: iocanel
*