Skip to content

Commit e327725

Browse files
committed
Fix storing unhashable subclasses into the __subclasses__ dict
1 parent b907afd commit e327725

File tree

2 files changed

+13
-1
lines changed

2 files changed

+13
-1
lines changed

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/EconomicMapStorage.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,11 @@ public void putUncached(Object key, Object value) {
165165
PutNode.putUncached(map, key, PyObjectHashNode.executeUncached(key), value);
166166
}
167167

168+
@TruffleBoundary
169+
public void putUncached(Object key, long hash, Object value) {
170+
PutNode.putUncached(map, key, hash, value);
171+
}
172+
168173
@TruffleBoundary
169174
public void putUncached(int key, Object value) {
170175
PutNode.putUncached(map, key, PyObjectHashNode.hash(key), value);

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/TypeNodes.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -814,6 +814,9 @@ public final List<KeyAndHash> execute(Frame frame, Node inliningTarget, HashingS
814814
Object subclassValue = pref.getObject();
815815
if (subclassValue == null) {
816816
Object key = HashingStorageIteratorKey.executeUncached(storage, it);
817+
// CPython uses the object pointer as the key, but we cannot so we do not
818+
// really know how the hash is arrived at and need to make sure to remove
819+
// the same hash that was stored.
817820
long hash = HashingStorageIteratorKeyHash.executeUncached(storage, it);
818821
acc.add(new KeyAndHash(key, hash));
819822
}
@@ -833,6 +836,10 @@ static void clearEmptyReferences(EconomicMapStorage storage) {
833836
protected static void addSubclass(PythonAbstractClass base, PythonManagedClass subclass) {
834837
CompilerAsserts.neverPartOfCompilation();
835838
PDict dict = executeUncached(base);
839+
// CPython uses the object pointer as the key, but we cannot since identity hashes are
840+
// not unique. So we need to use the actual weakref as the key here, but not all
841+
// classes are hashable. So we use the identity hash code for the map storage hash.
842+
long hash = System.identityHashCode(subclass);
836843
HashingStorage storage = dict.getDictStorage();
837844
Object weakref = PFactory.createReferenceType(PythonLanguage.get(null), subclass);
838845
if (!(storage instanceof EconomicMapStorage)) {
@@ -842,7 +849,7 @@ protected static void addSubclass(PythonAbstractClass base, PythonManagedClass s
842849
} else {
843850
clearEmptyReferences((EconomicMapStorage) storage);
844851
}
845-
((EconomicMapStorage) storage).putUncached(weakref, weakref);
852+
((EconomicMapStorage) storage).putUncached(weakref, hash, weakref);
846853
}
847854

848855
static final class RemoveSubclassValue extends HashingStorageForEachCallback<PythonManagedClass> {

0 commit comments

Comments
 (0)