Infinite loop trying to delete circularly referent models

[Kenneth Arnold]

​ConceptNet has Assertions, RawAssertions, and Ratings (among other models). The relationship looks like:

RawAssertion:
  assertion = ForeignKey(Assertion, null=True)
Rating:
  assertion = ForeginKey(Assertion)
Assertion:
  canonical = ForeignKey(RawAssertion)

Deleting an Assertion runs into an infinite loop in CollectedObjects.ordered_keys. The second time through the loop, when dealing with model=Rating, it has no children, but we shouldn't be setting found=True, because there's still a cyclic dependency. We should be able to tell that because it's already in dealt_with.

I tried to come up with a test case for this, but I couldn't make it fail. You can certainly reproduce it by downloading ConceptNet yourself.

Attached patch fixes the obvious problem by causing it to raise the CyclicDependency exception properly. By "fix", I mean it gets to the real problem: there's a cycle. Eventually we'll need to look at the null flags on the relations to figure out where to break the cycle, but that's a separate issue.

[Russell Keith-Magee]

I can't work out a test case for this. At this point, even an extremely complex example drawn from ConceptNet that fails would be most helpful.

[Jacob]

I can't reproduce this either. However, I think I'm going to hold my nose and check it in for 1.0 -- the only harm it could cause is to raise more CyclicDependency than strictly necessary. That's better than infinite loops, at least, and it's easier to clean up in the future when we figure out the actual fix.

[Jacob]

(In [8807]) Fixed #8115: avoid a infiniate loop when collecting related objects for deletion.

I can't reproduce the original error leading to #8115 and the patch. However, the only harm this change could cause is to raise more CyclicDependency exceptions than strictly necessary. That's better than infinite loops, at least, and it's easier to clean up in the future when we figure out the actual fix.

[Jacob]

Milestone 1.0 deleted