Issue #3018 addressing code review comments
Former-commit-id:8ba6b2722d[formerlye327f5e0bf[formerly 85d9dd0158ab873e1e9b76805853c712bb0c7c2c]] Former-commit-id:e327f5e0bfFormer-commit-id:54951100af
This commit is contained in:
Nate Jensen
parent
ef02928b92
commit
f8f47a8bf7
3 changed files with 131 additions and 38 deletions
|
|
@ -20,15 +20,12 @@
|
|||
package com.raytheon.uf.viz.core.reflect;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.lang.reflect.Field;
|
||||
import java.util.Collections;
|
||||
import java.util.HashSet;
|
||||
import java.util.Set;
|
||||
|
||||
import org.eclipse.core.runtime.FileLocator;
|
||||
import org.eclipse.osgi.framework.internal.core.AbstractBundle;
|
||||
import org.eclipse.osgi.framework.internal.core.BundleRepository;
|
||||
import org.eclipse.osgi.framework.internal.core.Framework;
|
||||
import org.osgi.framework.Bundle;
|
||||
import org.osgi.framework.wiring.BundleWiring;
|
||||
import org.reflections.Reflections;
|
||||
|
|
@ -67,40 +64,8 @@ public class BundleReflections {
|
|||
public BundleReflections(Bundle bundle, Scanner scanner) throws IOException {
|
||||
ConfigurationBuilder cb = new ConfigurationBuilder();
|
||||
BundleWiring bundleWiring = bundle.adapt(BundleWiring.class);
|
||||
|
||||
/*
|
||||
* If BundleReflection's constructor is invoked on a thread other than
|
||||
* main/UI thread, then there is a possible deadlock if the application
|
||||
* shuts down while the bundleWiring.getClassloader() call below is
|
||||
* still going. The BundleRepository of the Framework is the primary
|
||||
* resource that is in contention in this deadlock scenario, due to the
|
||||
* BundleRepository being used as a synchronization lock both deep in
|
||||
* bundleWiring.getClassloader() and in Framework shutdown code.
|
||||
*
|
||||
* Therefore to avoid this deadlock, we attempt to get the
|
||||
* BundleRepository and synchronize against it, ensuring the call to
|
||||
* getClassLoader() can finish and then release synchronization locks.
|
||||
*
|
||||
* If we fail to get the BundleRepository due to access restrictions,
|
||||
* then we proceed onwards anyway because the odds of the application
|
||||
* shutting down at the same time as this is still running is low, and
|
||||
* even if that occurs, the odds are further reduced that the two
|
||||
* threads will synchronize against the BundleRepository at the same
|
||||
* time and deadlock.
|
||||
*/
|
||||
BundleRepository bundleRepo = null;
|
||||
if (bundle instanceof AbstractBundle) {
|
||||
try {
|
||||
AbstractBundle ab = (AbstractBundle) bundle;
|
||||
Field bundleRepoField = Framework.getField(Framework.class,
|
||||
BundleRepository.class, true);
|
||||
bundleRepo = (BundleRepository) bundleRepoField.get(ab
|
||||
.getFramework());
|
||||
} catch (Throwable t) {
|
||||
// intentionally log to console and proceed anyway
|
||||
t.printStackTrace();
|
||||
}
|
||||
}
|
||||
BundleRepository bundleRepo = BundleRepositoryGetter
|
||||
.getFrameworkBundleRepository(bundle);
|
||||
|
||||
if (bundleWiring != null) {
|
||||
if (bundleRepo != null) {
|
||||
|
|
@ -108,6 +73,11 @@ public class BundleReflections {
|
|||
cb.addClassLoader(bundleWiring.getClassLoader());
|
||||
}
|
||||
} else {
|
||||
/*
|
||||
* even if we couldn't get the bundle repository to sync
|
||||
* against, it's probably safe, see BundleRepositoryGetter
|
||||
* javadoc
|
||||
*/
|
||||
cb.addClassLoader(bundleWiring.getClassLoader());
|
||||
}
|
||||
cb.addUrls(FileLocator.getBundleFile(bundle).toURI().toURL());
|
||||
|
|
|
|||
|
|
@ -0,0 +1,104 @@
|
|||
/**
|
||||
* This software was developed and / or modified by Raytheon Company,
|
||||
* pursuant to Contract DG133W-05-CQ-1067 with the US Government.
|
||||
*
|
||||
* U.S. EXPORT CONTROLLED TECHNICAL DATA
|
||||
* This software product contains export-restricted data whose
|
||||
* export/transfer/disclosure is restricted by U.S. law. Dissemination
|
||||
* to non-U.S. persons whether in the United States or abroad requires
|
||||
* an export license or other authorization.
|
||||
*
|
||||
* Contractor Name: Raytheon Company
|
||||
* Contractor Address: 6825 Pine Street, Suite 340
|
||||
* Mail Stop B8
|
||||
* Omaha, NE 68106
|
||||
* 402.291.0100
|
||||
*
|
||||
* See the AWIPS II Master Rights File ("Master Rights File.pdf") for
|
||||
* further licensing information.
|
||||
**/
|
||||
package com.raytheon.uf.viz.core.reflect;
|
||||
|
||||
import java.lang.reflect.Field;
|
||||
|
||||
import org.eclipse.osgi.framework.internal.core.AbstractBundle;
|
||||
import org.eclipse.osgi.framework.internal.core.BundleRepository;
|
||||
import org.eclipse.osgi.framework.internal.core.Framework;
|
||||
import org.osgi.framework.Bundle;
|
||||
|
||||
/**
|
||||
* Utility class to get the BundleRepository object associated with a Bundle, to
|
||||
* potentially synchronize against that object.
|
||||
*
|
||||
* Specifically if a call to BundleWiring.getClassLoader() is invoked on a
|
||||
* thread other than main/UI thread, then there is a possible deadlock if the
|
||||
* application shuts down while the BundleWiring.getClassLoader() call is still
|
||||
* going. The BundleRepository of the Framework is the primary resource that is
|
||||
* in contention in this deadlock scenario, due to the BundleRepository being
|
||||
* used as a synchronization lock both deep in bundleWiring.getClassloader() and
|
||||
* in Framework shutdown code. The other resource used as a synchronization lock
|
||||
* and causing the deadlock is the BundleLoader associated with the bundle.
|
||||
*
|
||||
* Therefore to avoid this deadlock, if you are going to call
|
||||
* BundleWiring.getClassLoader() you should attempt to get the BundleRepository
|
||||
* and synchronize against it. This will ensure the call to getClassLoader() can
|
||||
* finish and then release synchronization locks of both the BundleRepository
|
||||
* and BundleLoader.
|
||||
*
|
||||
* If we fail to get the BundleRepository due to access restrictions, then you
|
||||
* should proceed onwards anyway because the odds of the application shutting
|
||||
* down at the same time as the call to BundleWiring.getClassLoader() is still
|
||||
* running is low. Even if that occurs, the odds are further reduced that the
|
||||
* two threads will synchronize against the BundleRepository at the same time
|
||||
* and deadlock.
|
||||
*
|
||||
*
|
||||
* <pre>
|
||||
*
|
||||
* SOFTWARE HISTORY
|
||||
*
|
||||
* Date Ticket# Engineer Description
|
||||
* ------------ ---------- ----------- --------------------------
|
||||
* Apr 17, 2014 njensen Initial creation
|
||||
*
|
||||
* </pre>
|
||||
*
|
||||
* @author njensen
|
||||
* @version 1.0
|
||||
*/
|
||||
|
||||
public class BundleRepositoryGetter {
|
||||
|
||||
private BundleRepositoryGetter() {
|
||||
|
||||
}
|
||||
|
||||
/**
|
||||
* Attempts to retrieve the BundleRepository associated with the bundle's
|
||||
* framework. Returns the BundleRepository or null if it could not be
|
||||
* retrieved.
|
||||
*
|
||||
* @param bundle
|
||||
* the bundle to retrieve the associated BundleRepository for
|
||||
* @return the BundleRepository or null
|
||||
*/
|
||||
@SuppressWarnings("restriction")
|
||||
protected static BundleRepository getFrameworkBundleRepository(Bundle bundle) {
|
||||
BundleRepository bundleRepo = null;
|
||||
if (bundle instanceof AbstractBundle) {
|
||||
try {
|
||||
AbstractBundle ab = (AbstractBundle) bundle;
|
||||
Field bundleRepoField = Framework.getField(Framework.class,
|
||||
BundleRepository.class, true);
|
||||
bundleRepo = (BundleRepository) bundleRepoField.get(ab
|
||||
.getFramework());
|
||||
} catch (Throwable t) {
|
||||
// intentionally log to console and proceed anyway
|
||||
t.printStackTrace();
|
||||
}
|
||||
}
|
||||
|
||||
return bundleRepo;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
@ -28,6 +28,7 @@ import java.util.HashSet;
|
|||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
|
||||
import org.eclipse.osgi.framework.internal.core.BundleRepository;
|
||||
import org.osgi.framework.Bundle;
|
||||
import org.osgi.framework.namespace.BundleNamespace;
|
||||
import org.osgi.framework.namespace.PackageNamespace;
|
||||
|
|
@ -56,6 +57,7 @@ import com.raytheon.uf.viz.core.Activator;
|
|||
* Dec 10, 2013 2602 bsteffen Add null checks to detect unloaded
|
||||
* bundles.
|
||||
* Feb 03, 2013 2764 bsteffen Use OSGi API to get dependencies.
|
||||
* Apr 17, 2014 3018 njensen Synchronize against BundleRepository
|
||||
*
|
||||
* </pre>
|
||||
*
|
||||
|
|
@ -95,6 +97,7 @@ public class SubClassLocator implements ISubClassLocator {
|
|||
* @param base
|
||||
* @return
|
||||
*/
|
||||
@Override
|
||||
public Collection<Class<?>> locateSubClasses(Class<?> base) {
|
||||
Map<String, Set<Class<?>>> recursiveClasses = new HashMap<String, Set<Class<?>>>(
|
||||
bundleLookup.size(), 1.0f);
|
||||
|
|
@ -109,6 +112,7 @@ public class SubClassLocator implements ISubClassLocator {
|
|||
/**
|
||||
* Store the cache to disk.
|
||||
*/
|
||||
@Override
|
||||
public void save() {
|
||||
cache.save();
|
||||
}
|
||||
|
|
@ -265,10 +269,25 @@ public class SubClassLocator implements ISubClassLocator {
|
|||
if (bundleWiring == null) {
|
||||
return Collections.emptySet();
|
||||
}
|
||||
ClassLoader loader = bundleWiring.getClassLoader();
|
||||
|
||||
BundleRepository bundleRepo = BundleRepositoryGetter
|
||||
.getFrameworkBundleRepository(bundle);
|
||||
ClassLoader loader = null;
|
||||
if (bundleRepo != null) {
|
||||
synchronized (bundleRepo) {
|
||||
loader = bundleWiring.getClassLoader();
|
||||
}
|
||||
} else {
|
||||
/*
|
||||
* even if we couldn't get the bundle repository to sync against,
|
||||
* it's probably safe, see BundleRepositoryGetter javadoc
|
||||
*/
|
||||
loader = bundleWiring.getClassLoader();
|
||||
}
|
||||
if (loader == null) {
|
||||
return Collections.emptySet();
|
||||
}
|
||||
|
||||
HashSet<Class<?>> result = new HashSet<Class<?>>(classNames.size(),
|
||||
1.0f);
|
||||
for (String className : classNames) {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue