You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@river.apache.org by pe...@apache.org on 2011/12/13 11:36:44 UTC
svn commit: r1213641 - in /river/jtsk/trunk/src:
com/sun/jini/action/GetPropertyAction.java
net/jini/loader/pref/PreferredClassProvider.java
Author: peter_firmstone
Date: Tue Dec 13 10:36:43 2011
New Revision: 1213641
URL: http://svn.apache.org/viewvc?rev=1213641&view=rev
Log:
River-265 Fix for unlucky caching as requested.
River-401 Changed to utilise URI in place of URL in map's and arrays to avoid unnecessary DNS lookups.
Modified:
river/jtsk/trunk/src/com/sun/jini/action/GetPropertyAction.java
river/jtsk/trunk/src/net/jini/loader/pref/PreferredClassProvider.java
Modified: river/jtsk/trunk/src/com/sun/jini/action/GetPropertyAction.java
URL: http://svn.apache.org/viewvc/river/jtsk/trunk/src/com/sun/jini/action/GetPropertyAction.java?rev=1213641&r1=1213640&r2=1213641&view=diff
==============================================================================
--- river/jtsk/trunk/src/com/sun/jini/action/GetPropertyAction.java (original)
+++ river/jtsk/trunk/src/com/sun/jini/action/GetPropertyAction.java Tue Dec 13 10:36:43 2011
@@ -55,7 +55,7 @@ import net.jini.security.Security;
* @see Security
* @since 2.0
**/
-public class GetPropertyAction implements PrivilegedAction {
+public class GetPropertyAction implements PrivilegedAction<String> {
private static final Logger logger =
Logger.getLogger("com.sun.jini.action.GetPropertyAction");
@@ -98,7 +98,7 @@ public class GetPropertyAction implement
* @return the string value of the system property or the default
* value, or <code>null</code>
**/
- public Object run() {
+ public String run() {
try {
String value = System.getProperty(theProp);
if (value != null) {
Modified: river/jtsk/trunk/src/net/jini/loader/pref/PreferredClassProvider.java
URL: http://svn.apache.org/viewvc/river/jtsk/trunk/src/net/jini/loader/pref/PreferredClassProvider.java?rev=1213641&r1=1213640&r2=1213641&view=diff
==============================================================================
--- river/jtsk/trunk/src/net/jini/loader/pref/PreferredClassProvider.java (original)
+++ river/jtsk/trunk/src/net/jini/loader/pref/PreferredClassProvider.java Tue Dec 13 10:36:43 2011
@@ -29,6 +29,8 @@ import java.lang.ref.WeakReference;
import java.lang.reflect.Modifier;
import java.lang.reflect.Proxy;
import java.net.MalformedURLException;
+import java.net.URI;
+import java.net.URISyntaxException;
import java.net.URL;
import java.net.URLConnection;
import java.net.URLClassLoader;
@@ -37,6 +39,7 @@ import java.rmi.server.RMIClassLoaderSpi
import java.security.AccessController;
import java.security.CodeSource;
import java.security.Permission;
+import java.security.PermissionCollection;
import java.security.Permissions;
import java.security.PrivilegedAction;
import java.util.Arrays;
@@ -246,8 +249,8 @@ public class PreferredClassProvider exte
*/
private static String codebaseProperty = null;
static {
- String prop = (String) AccessController.doPrivileged(
- new GetPropertyAction("java.rmi.server.codebase"));
+ String prop = AccessController.doPrivileged(
+ new GetPropertyAction("java.rmi.server.codebase"));
if (prop != null && prop.trim().length() > 0) {
codebaseProperty = prop;
}
@@ -279,7 +282,7 @@ public class PreferredClassProvider exte
private final Map<LoaderKey,LoaderEntryHolder> loaderTable = new HashMap<LoaderKey,LoaderEntryHolder>();
/** reference queue for cleared class loader entries */
- private final ReferenceQueue refQueue = new ReferenceQueue();
+ private final ReferenceQueue<ClassLoader> refQueue = new ReferenceQueue<ClassLoader>();
/**
* Creates a new <code>PreferredClassProvider</code>.
@@ -343,7 +346,8 @@ public class PreferredClassProvider exte
* Map to hold permissions needed to check the URLs of
* URLClassLoader objects.
*/
- private final Map classLoaderPerms = new WeakHashMap();
+ private final Map<ClassLoader,PermissionCollection> classLoaderPerms
+ = new WeakHashMap<ClassLoader,PermissionCollection>();
/*
* Check permissions to load from the specified loader. The
@@ -355,22 +359,22 @@ public class PreferredClassProvider exte
* no permissions are checked, because the caller could have used
* an empty codebase to achieve the same effect anyway.
*/
- private void checkLoader(ClassLoader loader, ClassLoader parent,
+ private void checkLoader(ClassLoader loader, ClassLoader parent, URI[] uris,
URL[] urls)
{
SecurityManager sm = System.getSecurityManager();
if ((sm != null) && (loader != null) && (loader != parent)) {
- assert urlsMatchLoaderAnnotation(urls, loader);
+ assert urlsMatchLoaderAnnotation(uris, loader);
if (loader.getClass() == PreferredClassLoader.class) {
((PreferredClassLoader) loader).checkPermissions();
} else {
- Permissions perms;
+ PermissionCollection perms;
synchronized (classLoaderPerms) {
- perms = (Permissions) classLoaderPerms.get(loader);
+ perms = classLoaderPerms.get(loader);
if (perms == null) {
perms = new Permissions();
PreferredClassLoader.addPermissionsForURLs(
@@ -378,11 +382,12 @@ public class PreferredClassProvider exte
classLoaderPerms.put(loader, perms);
}
}
-
- Enumeration en = perms.elements();
- while (en.hasMoreElements()) {
- sm.checkPermission((Permission) en.nextElement());
- }
+ synchronized (perms){
+ Enumeration en = perms.elements();
+ while (en.hasMoreElements()) {
+ sm.checkPermission((Permission) en.nextElement());
+ }
+ }
}
}
}
@@ -482,8 +487,13 @@ public class PreferredClassProvider exte
});
}
- URL[] codebaseURLs = pathToURLs(codebase); // may be null
+ URI[] codebaseURIs = pathToURIs(codebase); // may be null
// throws MalformedURLException
+ int l = codebaseURIs.length;
+ URL[] codebaseURLs = new URL[l];
+ for (int i = 0; i < l; i++ ){
+ codebaseURLs[i] = asURL(codebaseURIs[i]); // throws MalformedURLException
+ }
/*
* Process array class names.
@@ -510,8 +520,8 @@ public class PreferredClassProvider exte
*/
SecurityManager sm = System.getSecurityManager();
if (defaultLoader != null &&
- (sm == null || codebaseURLs == null ||
- urlsMatchLoaderAnnotation(codebaseURLs, defaultLoader)))
+ (sm == null || codebaseURIs == null ||
+ urlsMatchLoaderAnnotation(codebaseURIs, defaultLoader)))
{
try {
Class c = Class.forName(name, false, defaultLoader);
@@ -534,7 +544,9 @@ public class PreferredClassProvider exte
logger.log(Level.FINEST,
"(thread context class loader: {0})", contextLoader);
}
- ClassLoader codebaseLoader = lookupLoader(codebaseURLs, contextLoader);
+ ClassLoader codebaseLoader;
+ codebaseLoader = lookupLoader(codebaseURIs, codebaseURLs, contextLoader);
+
/*
* Try remaining defaultLoader cases that don't require
@@ -562,7 +574,7 @@ public class PreferredClassProvider exte
SecurityException secEx = null;
if (sm != null) {
try {
- checkLoader(codebaseLoader, contextLoader, codebaseURLs);
+ checkLoader(codebaseLoader, contextLoader, codebaseURIs, codebaseURLs);
} catch (SecurityException e) {
secEx = e;
}
@@ -887,8 +899,13 @@ public class PreferredClassProvider exte
throws MalformedURLException
{
checkInitialized();
- URL[] codebaseURLs = pathToURLs(codebase); // may be null
+ URI[] codebaseURIs = pathToURIs(codebase); // may be null
// throws MalformedURLException
+ int l = codebaseURIs.length;
+ URL[] codebaseURLs = new URL[l];
+ for (int i = 0; i < l; i++ ){
+ codebaseURLs[i] = asURL(codebaseURIs[i]); // throws MalformedURLException
+ }
ClassLoader contextLoader = getRMIContextClassLoader();
SecurityManager sm = System.getSecurityManager();
@@ -898,8 +915,9 @@ public class PreferredClassProvider exte
return contextLoader;
}
- ClassLoader codebaseLoader = lookupLoader(codebaseURLs, contextLoader);
- checkLoader(codebaseLoader, contextLoader, codebaseURLs);
+ ClassLoader codebaseLoader;
+ codebaseLoader = lookupLoader(codebaseURIs, codebaseURLs, contextLoader);
+ checkLoader(codebaseLoader, contextLoader, codebaseURIs, codebaseURLs);
return codebaseLoader;
}
@@ -1059,9 +1077,13 @@ public class PreferredClassProvider exte
defaultLoader
});
}
-
- URL[] codebaseURLs = pathToURLs(codebase); // may be null
- // throws MalformedURLException
+ // throws MalformedURLException containing URISyntaxException message
+ URI[] codebaseURIs = pathToURIs(codebase); // may be null
+ int l = codebaseURIs.length;
+ URL[] codebaseURLs = new URL[l];
+ for (int i = 0; i < l; i++ ){
+ codebaseURLs[i] = asURL(codebaseURIs[i]); // throws MalformedURLException
+ }
/*
* Determine the codebase loader.
@@ -1071,7 +1093,8 @@ public class PreferredClassProvider exte
logger.log(Level.FINEST,
"(thread context class loader: {0})", contextLoader);
}
- ClassLoader codebaseLoader = lookupLoader(codebaseURLs, contextLoader);
+ ClassLoader codebaseLoader;
+ codebaseLoader = lookupLoader(codebaseURIs, codebaseURLs, contextLoader);
/*
* Check permission to access the codebase loader.
@@ -1080,7 +1103,7 @@ public class PreferredClassProvider exte
SecurityException secEx = null;
if (sm != null) {
try {
- checkLoader(codebaseLoader, contextLoader, codebaseURLs);
+ checkLoader(codebaseLoader, contextLoader, codebaseURIs, codebaseURLs);
} catch (SecurityException e) {
secEx = e;
}
@@ -1093,10 +1116,10 @@ public class PreferredClassProvider exte
boolean codebaseMatchesDL = false;
boolean tryDL =
sm == null || secEx != null ||
- codebaseURLs == null;
+ codebaseURIs == null;
if (!tryDL) {
codebaseMatchesDL =
- urlsMatchLoaderAnnotation(codebaseURLs, defaultLoader);
+ urlsMatchLoaderAnnotation(codebaseURIs, defaultLoader);
tryDL = codebaseMatchesDL ||
!(codebaseLoader instanceof PreferredClassLoader) ||
!interfacePreferred((PreferredClassLoader) codebaseLoader,
@@ -1316,19 +1339,19 @@ public class PreferredClassProvider exte
* for the specified class loader, or null if the annotation
* string is null.
**/
- private URL[] getLoaderAnnotationURLs(ClassLoader loader)
+ private URI[] getLoaderAnnotationURIs(ClassLoader loader)
throws MalformedURLException
{
- return pathToURLs(getLoaderAnnotation(loader, false));
+ return pathToURIs(getLoaderAnnotation(loader, false));
}
/**
* Returns true if the specified path of URLs is equal to the
* annotation URLs of the specified loader, and false otherwise.
**/
- private boolean urlsMatchLoaderAnnotation(URL[] urls, ClassLoader loader) {
+ private boolean urlsMatchLoaderAnnotation(URI[] urls, ClassLoader loader) {
try {
- return Arrays.equals(urls, getLoaderAnnotationURLs(loader));
+ return Arrays.equals(urls, getLoaderAnnotationURIs(loader));
} catch (MalformedURLException e) {
return false;
}
@@ -1395,20 +1418,25 @@ public class PreferredClassProvider exte
* @throws MalformedURLException if the string path of urls contains a
* mal-formed url which can not be converted into a url object.
*/
- private static URL[] pathToURLs(String path) throws MalformedURLException {
+ private static URI[] pathToURIs(String path) throws MalformedURLException {
if (path == null) {
return null;
}
synchronized (pathToURLsCache) {
Object[] v = (Object[]) pathToURLsCache.get(path);
if (v != null) {
- return ((URL[])v[0]);
+ return ((URI[])v[0]);
}
}
StringTokenizer st = new StringTokenizer(path); // divide by spaces
- URL[] urls = new URL[st.countTokens()];
+ URI[] urls = new URI[st.countTokens()];
for (int i = 0; st.hasMoreTokens(); i++) {
- urls[i] = new URL(st.nextToken());
+ try {
+ urls[i] = new URI(st.nextToken()).normalize();
+ } catch (URISyntaxException ex) {
+ throw new MalformedURLException("URL's must be RFC 2396 Compliant: "
+ + ex.getMessage());
+ }
}
synchronized (pathToURLsCache) {
pathToURLsCache.put(path,
@@ -1416,8 +1444,21 @@ public class PreferredClassProvider exte
}
return urls;
}
+
+ /* Converts a URI to an URL.
+ */
+ private URL asURL(URI uri) throws MalformedURLException{
+ if (uri == null) throw new NullPointerException("URI cannot be null");
+ try {
+ return uri.toURL();
+ } catch (IllegalArgumentException ex){
+ throw new MalformedURLException(ex.getMessage());
+ }
+ }
- /** map from weak(key=string) to [URL[], soft(key)] */
+ /** map from weak(key=string) to [URL[], soft(key)]
+ * A Soft equality based hash map is what's required here.
+ */
private static Map pathToURLsCache = new WeakHashMap(5);
/**
@@ -1429,12 +1470,13 @@ public class PreferredClassProvider exte
* The current implementation simply uses the current thread's
* context class loader.
*/
- return (ClassLoader) AccessController.doPrivileged(
- new PrivilegedAction() {
- public Object run() {
- return Thread.currentThread().getContextClassLoader();
- }
- });
+ return AccessController.doPrivileged(
+ new PrivilegedAction<ClassLoader>() {
+ public ClassLoader run() {
+ return Thread.currentThread().getContextClassLoader();
+ }
+ }
+ );
}
/**
@@ -1482,25 +1524,26 @@ public class PreferredClassProvider exte
* loader for a loader that has an export path which matches the
* parameter path.
*/
- private ClassLoader findOriginLoader(final URL[] pathURLs,
+ private ClassLoader findOriginLoader(final URI[] pathURLs,
final ClassLoader parent)
{
- return (ClassLoader) AccessController.doPrivileged(
- new PrivilegedAction() {
- public Object run() {
- return findOriginLoader0(pathURLs, parent);
- }
- });
+ return AccessController.doPrivileged(
+ new PrivilegedAction<ClassLoader>() {
+ public ClassLoader run() {
+ return findOriginLoader0(pathURLs, parent);
+ }
+ }
+ );
}
- private ClassLoader findOriginLoader0(URL[] pathURLs, ClassLoader parent) {
+ private ClassLoader findOriginLoader0(URI[] pathURLs, ClassLoader parent) {
for (ClassLoader ancestor = parent;
ancestor != null;
ancestor = ancestor.getParent())
{
- URL[] ancestorURLs;
+ URI[] ancestorURLs;
try {
- ancestorURLs = getLoaderAnnotationURLs(ancestor);
+ ancestorURLs = getLoaderAnnotationURIs(ancestor);
} catch (MalformedURLException e) {
// this ancestor's annotation must not match pathURLs
continue;
@@ -1531,15 +1574,15 @@ public class PreferredClassProvider exte
* and the given parent class loader. A new class loader instance
* will be created and returned if no match is found.
*/
- private ClassLoader lookupLoader(final URL[] urls,
- final ClassLoader parent)
+ private ClassLoader lookupLoader(final URI[] uris, URL[] urls,
+ final ClassLoader parent) throws MalformedURLException
{
/*
* If the requested codebase is null, then PreferredClassProvider
* assumes that the class is expected to be a "platform" class
* with respect to the parent class loader.
*/
- if (urls == null) {
+ if (uris == null) {
return parent;
}
@@ -1574,7 +1617,7 @@ public class PreferredClassProvider exte
}
}
- LoaderKey key = new LoaderKey(urls, parent);
+ LoaderKey key = new LoaderKey(uris, parent);
LoaderEntryHolder holder;
synchronized (loaderTable) {
if (!toRemove.isEmpty()) {
@@ -1609,7 +1652,7 @@ public class PreferredClassProvider exte
LoaderEntry entry = holder.entry;
ClassLoader loader;
if (entry == null ||
- (loader = (ClassLoader) entry.get()) == null)
+ (loader = entry.get()) == null)
{
/*
* If entry was in table but it's weak reference was cleared,
@@ -1637,18 +1680,22 @@ public class PreferredClassProvider exte
* context restricted to the permissions necessary to
* load classes from its codebase URL path.
*/
- loader = findOriginLoader(urls, parent);
+ loader = findOriginLoader(uris, parent);
if (loader == null) {
loader = createClassLoader(urls, parent, requireDlPerm);
+ /* RIVER-265
+ * The next section of code has been moved inside this
+ * block to avoid caching loaders found using
+ * findOriginLoader
+ *
+ * Finally, create an entry to hold the new loader with a
+ * weak reference and store it in the table with the key.
+ */
+ entry = new LoaderEntry(key, loader);
+ holder.entry = entry;
}
- /*
- * Finally, create an entry to hold the new loader with a
- * weak reference and store it in the table with the key.
- */
- entry = new LoaderEntry(key, loader);
- holder.entry = entry;
}
return loader;
}
@@ -1691,9 +1738,9 @@ public class PreferredClassProvider exte
final boolean requireDlPerm)
{
checkInitialized();
- return (ClassLoader)
- AccessController.doPrivileged(new PrivilegedAction() {
- public Object run() {
+ return
+ AccessController.doPrivileged(new PrivilegedAction<ClassLoader>() {
+ public ClassLoader run() {
return new PreferredClassLoader(urls, parent, null,
requireDlPerm);
}
@@ -1701,24 +1748,23 @@ public class PreferredClassProvider exte
}
/**
- * Loader table key: a codebase URL path and a weak reference to
+ * Loader table key: a codebase URI path and a weak reference to
* a parent class loader (possibly null). The weak reference is
* registered with "refQueue" so that the entry can be removed
* after the loader has become unreachable.
**/
- private class LoaderKey extends WeakReference {
- private final URL[] urls;
+ private class LoaderKey extends WeakReference<ClassLoader> {
+ private final URI[] uris;
private final boolean nullParent;
private final int hashValue;
- public LoaderKey(URL[] urls, ClassLoader parent) {
+ public LoaderKey(URI[] urls, ClassLoader parent) {
super(parent, refQueue);
- this.urls = urls;
nullParent = (parent == null);
-
+ uris = urls;
int h = nullParent ? 0 : parent.hashCode();
for (int i = 0; i < urls.length; i++) {
- h ^= urls[i].hashCode();
+ h ^= uris[i].hashCode();
}
hashValue = h;
}
@@ -1736,9 +1782,9 @@ public class PreferredClassProvider exte
LoaderKey other = (LoaderKey) obj;
ClassLoader parent;
return (nullParent ? other.nullParent
- : ((parent = (ClassLoader) get()) != null &&
+ : ((parent = get()) != null &&
parent == other.get()))
- && Arrays.equals(urls, other.urls);
+ && Arrays.equals(uris, other.uris);
}
}
@@ -1747,13 +1793,21 @@ public class PreferredClassProvider exte
* weak reference is registered with "refQueue" so that the entry
* can be removed after the loader has become unreachable.
**/
- private class LoaderEntry extends WeakReference {
+ private class LoaderEntry extends WeakReference<ClassLoader> {
public final LoaderKey key; // for efficient removal
/**
* set to true if the entry has been removed from the table
* because it has been replaced, so it should not be attempted
* to be removed again
+ *
+ * REMINDER: 13th Dec 2011 - Peter Firmstone Commented:
+ * removed is mutated while holding LoaderEntryHolder's
+ * lock, once it becomes unreachable, the queue accesses this variable
+ * without synchronisation or volatility, by the time this object
+ * is ready for collection , it is unlikely that it's state will be
+ * modified. If however at some point we change PreferredClassProvider's
+ * synchronisation strategy we need to revisit this.
*/
public boolean removed = false;
@@ -1767,9 +1821,9 @@ public class PreferredClassProvider exte
}
private static ClassLoader getClassLoader(final Class c) {
- return (ClassLoader) AccessController.doPrivileged(
- new PrivilegedAction() {
- public Object run() { return c.getClassLoader(); }
+ return AccessController.doPrivileged(
+ new PrivilegedAction<ClassLoader>() {
+ public ClassLoader run() { return c.getClassLoader(); }
});
}
}