Opened 12 years ago

Closed 10 years ago

Last modified 10 years ago

#18541 closed Bug (duplicate)

Infinite Lock With Caching Backend

Reported by: zmsmith Owned by: nobody
Component: Core (Cache system) Version: 1.4
Severity: Normal Keywords: caching
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Basically, if you try to cache an object with a __getstate__ method that hits the cache, the LocMem backend will enter an infinite lock.

I discovered this when a QuerySet that was using a caching manager was being written to the cache and pickling forced the QuerySet to evaluate and interact with the cache.

The test I added covers this scenario, but unfortunately the failure condition is an infinite lock that will just hang and give no feedback about the failing test. I didn't know if django has any convention for timing out tests, so I didn't try to create one.

Attachments (2)

LocMemlocking.diff (1.9 KB ) - added by zmsmith 12 years ago.
LocMemlocking.diff
LocMem.diff (2.1 KB ) - added by zmsmith 12 years ago.
Patch with improved test

Download all attachments as: .zip

Change History (9)

by zmsmith, 12 years ago

Attachment: LocMemlocking.diff added

LocMemlocking.diff

comment:1 by Aymeric Augustin, 12 years ago

Component: UncategorizedCore (Cache system)
Patch needs improvement: set
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

Historically, a one-liner put in the cache a deepcopy of the object. Later deepcopy was replaced by pickle, and the code was refactored to split the two operations:

  • creating acopy of the object
  • storing it in the cache

Only the second part needs to be thread-safe. I agree with the change.

An test that hangs infinitely isn't helpful. Maybe we could put a timeout with signal.alarm()? Any better ideas?

comment:2 by zmsmith, 12 years ago

Had a chance to look at this, but signal.alarm() is not cooperating with the thread locking.

Interestingly, when this lock occurs, SIGINT is also just getting swallowed. I wonder if the code in django.utils.synch is doing something a little too aggressive, but it's hard for me to say because I don't have any real experience with threading.

by zmsmith, 12 years ago

Attachment: LocMem.diff added

Patch with improved test

comment:3 by zmsmith, 12 years ago

I changed the test to not create the locking behavior and fail correctly against the old code. New patch is uploaded.

comment:4 by FrankBie, 11 years ago

Needs documentation: set

Testcode and your patch needs some inline comments

comment:5 by Tim Graham, 11 years ago

Pull request exists as well (I think it's the same as the patch attached to the ticket): https://github.com/django/django/pull/694

comment:6 by Tim Graham, 10 years ago

Resolution: duplicate
Status: newclosed

Duplicate of #20613, I'll merge the test that's included in this PR though as tests weren't added with that ticket.

comment:7 by Tim Graham <timograham@…>, 10 years ago

In 6c5a30b4e7f51e8c255dc104714d5748e5b5870c:

Added tests for LocalMemCache deadlocks. refs #20613 and refs #18541.

Thanks Zach Smith for the patch.

Note: See TracTickets for help on using tickets.
Back to Top