Skip to content

Commit 8b9a39e

Browse files
committed
Make sure we throw an exception if we attempt to hash an MRTG more than once.
Somehow, we're seeing cases where the graph appears to be unstable and so we get cycles in the MRTGs which is not supposed to happen. In the future, we need to implement something else to ensure that if this happens we can collapse the groups. But really they're supposed to be stable, so we should try to figure out why the python object graph is not stable.
1 parent 94842f0 commit 8b9a39e

File tree

2 files changed

+45
-24
lines changed

2 files changed

+45
-24
lines changed

typed_python/MutuallyRecursiveTypeGroup.cpp

Lines changed: 40 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -929,34 +929,51 @@ void MutuallyRecursiveTypeGroup::computeHash() {
929929
return;
930930
}
931931

932-
// we are a recursive group head. We want to compute the hash
933-
// of all of our constituents where, when each of them looks at
934-
// other types within _our_ group, we simply hash in a placeholder
935-
// for the position.
936-
ShaHash wholeGroupHash;
937-
938-
for (auto idAndType: mIndexToObject) {
939-
Type* t = idAndType.second.type();
940-
941-
if (t) {
942-
// this actually walks the type and computes a sha-hash based on
943-
// what's inside the type one layer down.
944-
wholeGroupHash += t->computeIdentityHash(this);
945-
946-
if (t->isRecursive()) {
947-
wholeGroupHash += ShaHash(t->name());
932+
if (mIsCurrentlyHashing) {
933+
throw std::runtime_error(
934+
"Somehow we are already computing the hash of this MRTG. "
935+
"This means that when we computed the group's constituents, we missed "
936+
"a link between elements of this group and elements of a calling group."
937+
);
938+
}
939+
940+
try {
941+
// mark that we're currently hashing.
942+
mIsCurrentlyHashing = true;
943+
944+
// we are a recursive group head. We want to compute the hash
945+
// of all of our constituents where, when each of them looks at
946+
// other types within _our_ group, we simply hash in a placeholder
947+
// for the position.
948+
ShaHash wholeGroupHash;
949+
950+
for (auto idAndType: mIndexToObject) {
951+
Type* t = idAndType.second.type();
952+
953+
if (t) {
954+
// this actually walks the type and computes a sha-hash based on
955+
// what's inside the type one layer down.
956+
wholeGroupHash += t->computeIdentityHash(this);
957+
958+
if (t->isRecursive()) {
959+
wholeGroupHash += ShaHash(t->name());
960+
}
961+
} else {
962+
wholeGroupHash += computePyObjectShaHash(idAndType.second.pyobj(), this);
948963
}
949-
} else {
950-
wholeGroupHash += computePyObjectShaHash(idAndType.second.pyobj(), this);
951964
}
952-
}
953965

954-
mHash = wholeGroupHash;
966+
mHash = wholeGroupHash;
967+
mIsCurrentlyHashing = false;
955968

956-
PyEnsureGilAcquired getTheGil;
969+
PyEnsureGilAcquired getTheGil;
957970

958-
if (mHashToGroup.find(mHash) == mHashToGroup.end()) {
959-
mHashToGroup[mHash] = this;
971+
if (mHashToGroup.find(mHash) == mHashToGroup.end()) {
972+
mHashToGroup[mHash] = this;
973+
}
974+
} catch(...) {
975+
mIsCurrentlyHashing = false;
976+
throw;
960977
}
961978
}
962979

typed_python/MutuallyRecursiveTypeGroup.hpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@
2323

2424
class MutuallyRecursiveTypeGroup {
2525
public:
26-
MutuallyRecursiveTypeGroup() : mAnyPyObjectsIncorrectlyOrdered(false)
26+
MutuallyRecursiveTypeGroup() :
27+
mAnyPyObjectsIncorrectlyOrdered(false),
28+
mIsCurrentlyHashing(false)
2729
{}
2830

2931
MutuallyRecursiveTypeGroup(ShaHash hash);
@@ -126,6 +128,8 @@ class MutuallyRecursiveTypeGroup {
126128

127129
void computeHash();
128130

131+
bool mIsCurrentlyHashing;
132+
129133
ShaHash mHash;
130134

131135
std::map<int32_t, TypeOrPyobj> mIndexToObject;

0 commit comments

Comments
 (0)