Skip to content

Commit e19c737

Browse files
jimbakerjimbaker
committed
Fixed #1522 by weakening references in class_to_type map (both key and
value). However, in certain cases our code expects that these refs are in fact hard, so make all exposed types hard and also by default PyType#fromClass will add a hard ref. In contrast, types imported in that proxy Java classes should now be weak. Added Guava R05 (which includes google collections), to support the desired map. We may wish to strip this down, however, it could be useful in the future too.
1 parent d395f2e commit e19c737

9 files changed

Lines changed: 48 additions & 25 deletions

File tree

‎build.xml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@ svnant.jar.dir=${basedir}/../externals/svnant-jars
188188
<pathelement path="${extlibs.dir}/asm-3.1.jar" />
189189
<pathelement path="${extlibs.dir}/asm-commons-3.1.jar" />
190190
<pathelement path="${extlibs.dir}/constantine.jar" />
191+
<pathelement path="${extlibs.dir}/guava-r05.jar" />
191192
<pathelement path="${extlibs.dir}/jaffl.jar"/>
192193
<pathelement path="${extlibs.dir}/jffi-Darwin.jar"/>
193194
<pathelement path="${extlibs.dir}/jffi-i386-FreeBSD.jar"/>
@@ -584,6 +585,8 @@ The readme text for the next release will be like:
584585
<zipfileset src="extlibs/asm-commons-3.1.jar"/>
585586
<zipfileset src="extlibs/asm-util-3.1.jar"/>
586587
<rule pattern="org.objectweb.asm.**" result="org.python.objectweb.asm.@1"/>
588+
<zipfileset src="extlibs/guava-r05.jar"/>
589+
<rule pattern="com.google.**" result="org.python.google.@1"/>
587590
<zipfileset src="extlibs/jaffl.jar"/>
588591
<zipfileset src="extlibs/jffi-Darwin.jar"/>
589592
<zipfileset src="extlibs/jffi-i386-FreeBSD.jar"/>

‎extlibs/guava-r05.jar‎

911 KB
Binary file not shown.

‎src/org/python/core/PyJavaType.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public class PyJavaType extends PyType {
5959
private Set<String> modified;
6060

6161
public static PyObject wrapJavaObject(Object o) {
62-
PyObject obj = new PyObjectDerived(PyType.fromClass(o.getClass()));
62+
PyObject obj = new PyObjectDerived(PyType.fromClass(o.getClass(), false));
6363
obj.javaProxy = o;
6464
return obj;
6565
}

‎src/org/python/core/PyObject.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ public PyObject(PyType objtype) {
6363
* field to correspond to the specific subclass of <code>PyObject</code> being instantiated.
6464
**/
6565
public PyObject() {
66-
objtype = PyType.fromClass(getClass());
66+
objtype = PyType.fromClass(getClass(), false);
6767
}
6868

6969
/**

‎src/org/python/core/PyType.java‎

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
import org.python.modules._weakref.WeakrefModule;
2323
import org.python.util.Generic;
2424

25+
import com.google.common.collect.MapMaker;
26+
2527
/**
2628
* The Python Type object implementation.
2729
*/
@@ -97,7 +99,8 @@ public class PyType extends PyObject implements Serializable {
9799

98100
/** Mapping of Java classes to their PyTypes. */
99101
private static Map<Class<?>, PyType> class_to_type;
100-
102+
private static Set<PyType> exposedTypes;
103+
101104
/** Mapping of Java classes to their TypeBuilders. */
102105
private static Map<Class<?>, TypeBuilder> classToBuilder;
103106

@@ -121,7 +124,15 @@ public static PyObject type___new__(PyNewWrapper new_, boolean init, PyType subt
121124
PyObject[] args, String[] keywords) {
122125
// Special case: type(x) should return x.getType()
123126
if (args.length == 1 && keywords.length == 0) {
124-
return args[0].getType();
127+
PyObject obj = args[0];
128+
PyType objType = obj.getType();
129+
130+
// special case for PyStringMap so that it types as a dict
131+
PyType psmType = PyType.fromClass(PyStringMap.class);
132+
if (objType == psmType) {
133+
return PyDictionary.TYPE;
134+
}
135+
return objType;
125136
}
126137
// If that didn't trigger, we need 3 arguments. but ArgParser below may give a msg
127138
// saying type() needs exactly 3.
@@ -517,7 +528,7 @@ protected void init(Class<?> forClass, Set<PyJavaType> needsInners) {
517528
* followed by the mro of baseClass.
518529
*/
519530
protected void computeLinearMro(Class<?> baseClass) {
520-
base = PyType.fromClass(baseClass);
531+
base = PyType.fromClass(baseClass, false);
521532
mro = new PyType[base.mro.length + 1];
522533
System.arraycopy(base.mro, 0, mro, 1, base.mro.length);
523534
mro[0] = this;
@@ -622,7 +633,7 @@ private void setupProxy(Class<?> baseProxyClass, List<Class<?>> interfaces) {
622633
dict);
623634
javaProxy = proxyClass;
624635

625-
PyType proxyType = PyType.fromClass(proxyClass);
636+
PyType proxyType = PyType.fromClass(proxyClass, false);
626637
List<PyObject> cleanedBases = Generic.list();
627638
boolean addedProxyType = false;
628639
for (PyObject base : bases) {
@@ -908,7 +919,7 @@ PyObject[] computeMro() {
908919

909920
PyObject[] computeMro(MROMergeState[] toMerge, List<PyObject> mro) {
910921
boolean addedProxy = false;
911-
PyType proxyAsType = javaProxy == null ? null : PyType.fromClass(((Class<?>)javaProxy));
922+
PyType proxyAsType = javaProxy == null ? null : PyType.fromClass(((Class<?>)javaProxy), false);
912923
scan : for (int i = 0; i < toMerge.length; i++) {
913924
if (toMerge[i].isMerged()) {
914925
continue scan;
@@ -1164,7 +1175,7 @@ public PyObject super_lookup(PyType ref, String name) {
11641175
return null;
11651176
}
11661177

1167-
public static void addBuilder(Class<?> forClass, TypeBuilder builder) {
1178+
public synchronized static void addBuilder(Class<?> forClass, TypeBuilder builder) {
11681179
if (classToBuilder == null) {
11691180
classToBuilder = Generic.map();
11701181
}
@@ -1181,9 +1192,9 @@ public static void addBuilder(Class<?> forClass, TypeBuilder builder) {
11811192
}
11821193
}
11831194

1184-
private static PyType addFromClass(Class<?> c, Set<PyJavaType> needsInners) {
1195+
private synchronized static PyType addFromClass(Class<?> c, Set<PyJavaType> needsInners) {
11851196
if (ExposeAsSuperclass.class.isAssignableFrom(c)) {
1186-
PyType exposedAs = fromClass(c.getSuperclass());
1197+
PyType exposedAs = fromClass(c.getSuperclass(), false);
11871198
class_to_type.put(c, exposedAs);
11881199
return exposedAs;
11891200
}
@@ -1227,7 +1238,7 @@ private static TypeBuilder getBuilder(Class<?> c) {
12271238
return builder;
12281239
}
12291240

1230-
private static PyType createType(Class<?> c, Set<PyJavaType> needsInners) {
1241+
private synchronized static PyType createType(Class<?> c, Set<PyJavaType> needsInners) {
12311242
PyType newtype;
12321243
if (c == PyType.class) {
12331244
newtype = new PyType(false);
@@ -1251,18 +1262,25 @@ private static PyType createType(Class<?> c, Set<PyJavaType> needsInners) {
12511262
return newtype;
12521263
}
12531264

1254-
// XXX what's the proper scope of this synchronization? given module import lock, might be
1255-
// ok to omit this sync here...
1265+
// re the synchronization used here: this result is cached in each type obj,
1266+
// so we just need to prevent data races. all public methods that access class_to_type
1267+
// are themselves synchronized. However, if we use Google Collections/Guava,
1268+
// MapMaker only uses ConcurrentMap anyway
1269+
12561270
public static synchronized PyType fromClass(Class<?> c) {
1271+
return fromClass(c, true);
1272+
}
1273+
1274+
public static synchronized PyType fromClass(Class<?> c, boolean hardRef) {
12571275
if (class_to_type == null) {
1258-
class_to_type = Generic.synchronizedWeakHashMap();
1276+
class_to_type = new MapMaker().weakKeys().weakValues().makeMap();
12591277
addFromClass(PyType.class, null);
12601278
}
12611279
PyType type = class_to_type.get(c);
12621280
if (type != null) {
12631281
return type;
12641282
}
1265-
// We haven't seen this class before, so it's type needs to be created. If it's being
1283+
// We haven't seen this class before, so its type needs to be created. If it's being
12661284
// exposed as a Java class, defer processing its inner types until it's completely
12671285
// created in case the inner class references a class that references this class.
12681286
Set<PyJavaType> needsInners = Generic.set();
@@ -1284,10 +1302,17 @@ public static synchronized PyType fromClass(Class<?> c) {
12841302
|| ExposeAsSuperclass.class.isAssignableFrom(inner)) {
12851303
Py.BOOTSTRAP_TYPES.add(inner);
12861304
}
1287-
javaType.dict.__setitem__(inner.getSimpleName(), PyType.fromClass(inner));
1305+
javaType.dict.__setitem__(inner.getSimpleName(), PyType.fromClass(inner, hardRef));
12881306
}
12891307
}
12901308
}
1309+
if (hardRef && result != null) {
1310+
if (exposedTypes == null) {
1311+
exposedTypes = Generic.set();
1312+
}
1313+
exposedTypes.add(result) ;
1314+
}
1315+
12911316
return result;
12921317
}
12931318

@@ -1757,7 +1782,7 @@ static class TypeResolver implements Serializable {
17571782

17581783
private Object readResolve() {
17591784
if (underlying_class != null) {
1760-
return PyType.fromClass(underlying_class);
1785+
return PyType.fromClass(underlying_class, false);
17611786
}
17621787
PyObject mod = imp.importName(module.intern(), false);
17631788
PyObject pytyp = mod.__getattr__(name.intern());

‎src/org/python/core/adapter/ClassicPyObjectAdapter.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public PyObject adapt(Object o) {
7373
add(new ClassAdapter(Class.class) {
7474

7575
public PyObject adapt(Object o) {
76-
return PyType.fromClass((Class<?>)o);
76+
return PyType.fromClass((Class<?>)o, false);
7777
}
7878

7979
});

‎src/org/python/core/imp.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,7 @@ static PyObject createFromClass(String name, Class<?> c) {
399399
throw Py.JavaError(e);
400400
}
401401
}
402-
return PyType.fromClass(c); // xxx?
402+
return PyType.fromClass(c, false); // xxx?
403403
}
404404

405405
static PyObject getPathImporter(PyObject cache, PyList hooks, PyObject p) {

‎src/org/python/modules/zipimport/zipimporter.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ private void zipimporter___init__(String path) {
117117
zipimport._zip_directory_cache.__setitem__(archive, files);
118118
}
119119
} else {
120-
throw zipimport.ZipImportError("not a Zip file");
120+
throw zipimport.ZipImportError("not a Zip file: " + path);
121121
}
122122

123123
if (prefix != "" && !prefix.endsWith(File.separator)) {

‎src/org/python/util/Generic.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,6 @@ public static <K, V> Map<K, V> map() {
5858
return new HashMap<K, V>();
5959
}
6060

61-
public static <K, V> Map<K, V> synchronizedWeakHashMap() {
62-
return Collections.synchronizedMap(new WeakHashMap<K, V>());
63-
}
64-
65-
6661
/**
6762
* Makes a ConcurrentMap using generic types inferred from whatever this is being
6863
* assigned to.

0 commit comments

Comments
 (0)