Thanks for the explanation, Jarek. 2011/5/25 Jarek Gawor <jga...@gmail.com>: > Since we are tracking 2 states now (ACTIVE or STARTING) and if a > bundle does not have any faces configuration, returning null in > addingBundle() would cause addingBundle() to be called twice for the > same bundle. So the bundle could be scanned twice needlessly... which > probably would happen in most cases anyway. Returning non-null > guarantees that the bundle is scanned only once. That also means that > the BundleTracker is tracking every bundle but that is not a problem > in this case. > > Jarek > > On Wed, May 25, 2011 at 1:59 AM, Ivan <xhh...@gmail.com> wrote: >> Do we need to always return a non null value in the addingBundle >> method ? It looks to me that if no related files found in the target >> bundle, it is better to return null to tell the tracker to ignore it. >> Thanks. >> >> 2011/5/25 <ga...@apache.org>: >>> Author: gawor >>> Date: Wed May 25 05:25:34 2011 >>> New Revision: 1127388 >>> >>> URL: http://svn.apache.org/viewvc?rev=1127388&view=rev >>> Log: >>> GERONIMO-5938, GERONIMO-5976: Scan bundles for tag libs a little sooner to >>> prevent a potential race condition with WAB extender >>> >>> Modified: >>> >>> geronimo/server/trunk/plugins/jasper/geronimo-jasper/src/main/java/org/apache/geronimo/jasper/internal/Activator.java >>> >>> geronimo/server/trunk/plugins/jasper/geronimo-jasper/src/main/java/org/apache/geronimo/jasper/internal/TldRegistryImpl.java >>> >>> geronimo/server/trunk/plugins/myfaces/geronimo-myfaces/src/main/java/org/apache/geronimo/myfaces/config/resource/osgi/Activator.java >>> >>> geronimo/server/trunk/plugins/myfaces/geronimo-myfaces/src/main/java/org/apache/geronimo/myfaces/config/resource/osgi/ConfigBundleTrackerCustomizer.java >>> >>> Modified: >>> geronimo/server/trunk/plugins/jasper/geronimo-jasper/src/main/java/org/apache/geronimo/jasper/internal/Activator.java >>> URL: >>> http://svn.apache.org/viewvc/geronimo/server/trunk/plugins/jasper/geronimo-jasper/src/main/java/org/apache/geronimo/jasper/internal/Activator.java?rev=1127388&r1=1127387&r2=1127388&view=diff >>> ============================================================================== >>> --- >>> geronimo/server/trunk/plugins/jasper/geronimo-jasper/src/main/java/org/apache/geronimo/jasper/internal/Activator.java >>> (original) >>> +++ >>> geronimo/server/trunk/plugins/jasper/geronimo-jasper/src/main/java/org/apache/geronimo/jasper/internal/Activator.java >>> Wed May 25 05:25:34 2011 >>> @@ -33,7 +33,7 @@ public class Activator implements Bundle >>> TldRegistryImpl tldRegistry = new TldRegistryImpl(context); >>> tldRegistryRegistration = >>> context.registerService(TldRegistry.class.getName(), tldRegistry, null); >>> >>> - tldBundleTracker = new BundleTracker(context, Bundle.ACTIVE, >>> tldRegistry); >>> + tldBundleTracker = new BundleTracker(context, Bundle.STARTING | >>> Bundle.ACTIVE, tldRegistry); >>> tldBundleTracker.open(); >>> } >>> >>> >>> Modified: >>> geronimo/server/trunk/plugins/jasper/geronimo-jasper/src/main/java/org/apache/geronimo/jasper/internal/TldRegistryImpl.java >>> URL: >>> http://svn.apache.org/viewvc/geronimo/server/trunk/plugins/jasper/geronimo-jasper/src/main/java/org/apache/geronimo/jasper/internal/TldRegistryImpl.java?rev=1127388&r1=1127387&r2=1127388&view=diff >>> ============================================================================== >>> --- >>> geronimo/server/trunk/plugins/jasper/geronimo-jasper/src/main/java/org/apache/geronimo/jasper/internal/TldRegistryImpl.java >>> (original) >>> +++ >>> geronimo/server/trunk/plugins/jasper/geronimo-jasper/src/main/java/org/apache/geronimo/jasper/internal/TldRegistryImpl.java >>> Wed May 25 05:25:34 2011 >>> @@ -109,22 +109,12 @@ public class TldRegistryImpl implements >>> @Override >>> public Object addingBundle(Bundle bundle, BundleEvent event) { >>> Collection<TldProvider.TldEntry> tlds = scanBundle(bundle); >>> - if (tlds.isEmpty()) { >>> - return null; >>> - } else { >>> - map.put(bundle, tlds); >>> - return bundle; >>> - } >>> + map.put(bundle, tlds); >>> + return bundle; >>> } >>> >>> @Override >>> public void modifiedBundle(Bundle bundle, BundleEvent event, Object >>> object) { >>> - Collection<TldProvider.TldEntry> tlds = scanBundle(bundle); >>> - if (tlds.isEmpty()) { >>> - map.remove(bundle); >>> - } else { >>> - map.put(bundle, tlds); >>> - } >>> } >>> >>> @Override >>> @@ -134,7 +124,7 @@ public class TldRegistryImpl implements >>> >>> private Collection<TldProvider.TldEntry> scanBundle(Bundle bundle) { >>> ServiceReference reference = >>> bundleContext.getServiceReference(PackageAdmin.class.getName()); >>> - PackageAdmin packageAdmin = (PackageAdmin) >>> bundle.getBundleContext().getService(reference); >>> + PackageAdmin packageAdmin = (PackageAdmin) >>> bundleContext.getService(reference); >>> try { >>> BundleResourceFinder resourceFinder = new >>> BundleResourceFinder(packageAdmin, bundle, "META-INF/", ".tld"); >>> TldResourceFinderCallback callback = new >>> TldResourceFinderCallback(); >>> >>> Modified: >>> geronimo/server/trunk/plugins/myfaces/geronimo-myfaces/src/main/java/org/apache/geronimo/myfaces/config/resource/osgi/Activator.java >>> URL: >>> http://svn.apache.org/viewvc/geronimo/server/trunk/plugins/myfaces/geronimo-myfaces/src/main/java/org/apache/geronimo/myfaces/config/resource/osgi/Activator.java?rev=1127388&r1=1127387&r2=1127388&view=diff >>> ============================================================================== >>> --- >>> geronimo/server/trunk/plugins/myfaces/geronimo-myfaces/src/main/java/org/apache/geronimo/myfaces/config/resource/osgi/Activator.java >>> (original) >>> +++ >>> geronimo/server/trunk/plugins/myfaces/geronimo-myfaces/src/main/java/org/apache/geronimo/myfaces/config/resource/osgi/Activator.java >>> Wed May 25 05:25:34 2011 >>> @@ -62,7 +62,7 @@ public class Activator implements Bundle >>> // register this as a service >>> registryRegistration = >>> context.registerService(ConfigRegistry.class.getName(), registry, null); >>> >>> - bt = new BundleTracker(context, Bundle.ACTIVE, new >>> ConfigBundleTrackerCustomizer(this, context.getBundle(), registry)); >>> + bt = new BundleTracker(context, Bundle.STARTING | >>> Bundle.ACTIVE, new ConfigBundleTrackerCustomizer(this, context.getBundle(), >>> registry)); >>> bt.open(); >>> } >>> >>> >>> Modified: >>> geronimo/server/trunk/plugins/myfaces/geronimo-myfaces/src/main/java/org/apache/geronimo/myfaces/config/resource/osgi/ConfigBundleTrackerCustomizer.java >>> URL: >>> http://svn.apache.org/viewvc/geronimo/server/trunk/plugins/myfaces/geronimo-myfaces/src/main/java/org/apache/geronimo/myfaces/config/resource/osgi/ConfigBundleTrackerCustomizer.java?rev=1127388&r1=1127387&r2=1127388&view=diff >>> ============================================================================== >>> --- >>> geronimo/server/trunk/plugins/myfaces/geronimo-myfaces/src/main/java/org/apache/geronimo/myfaces/config/resource/osgi/ConfigBundleTrackerCustomizer.java >>> (original) >>> +++ >>> geronimo/server/trunk/plugins/myfaces/geronimo-myfaces/src/main/java/org/apache/geronimo/myfaces/config/resource/osgi/ConfigBundleTrackerCustomizer.java >>> Wed May 25 05:25:34 2011 >>> @@ -55,7 +55,8 @@ public class ConfigBundleTrackerCustomiz >>> if (bundle.equals(registryBundle)) { >>> return null; >>> } >>> - return registry.addBundle(bundle) ? Boolean.TRUE : null; >>> + registry.addBundle(bundle); >>> + return bundle; >>> } >>> >>> @Override >>> @@ -64,7 +65,7 @@ public class ConfigBundleTrackerCustomiz >>> } >>> >>> @Override >>> - public void removedBundle(Bundle bundle, BundleEvent event, Object >>> object) { >>> + public void removedBundle(Bundle bundle, BundleEvent event, Object >>> object) { >>> // have the registry process this >>> registry.removeBundle(bundle, object); >>> } >>> >>> >>> >> >> >> >> -- >> Ivan >> >
-- Ivan
