Django

Code

Ticket #8115 (closed: fixed)

Opened 4 months ago

Last modified 3 months ago

Infinite loop trying to delete circularly referent models

Reported by: kcarnold Assigned to: nobody
Milestone: 1.0 Component: Database layer (models, ORM)
Version: SVN Keywords: 1.0-blocker
Cc: kenneth.arnold@gmail.com Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

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.

Attachments

django-cyclic-dependency-8115.patch (0.5 kB) - added by kcarnold on 08/04/08 15:59:49.

Change History

08/04/08 15:59:49 changed by kcarnold

  • attachment django-cyclic-dependency-8115.patch added.

08/12/08 11:59:04 changed by jacob

  • needs_better_patch changed.
  • needs_docs changed.
  • stage changed from Unreviewed to Accepted.
  • needs_tests changed.
  • milestone set to 1.0.

08/27/08 11:28:44 changed by anonymous

  • has_patch set to 1.

08/31/08 15:48:01 changed by jacob

  • keywords set to 1.0-blocker.

09/01/08 08:11:34 changed by russellm

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.

09/01/08 14:32:25 changed by 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.

09/01/08 14:35:06 changed by jacob

  • status changed from new to closed.
  • resolution set to fixed.

(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.

Add/Change #8115 (Infinite loop trying to delete circularly referent models)




Change Properties
Action