#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)
Change History (9)
by , 12 years ago
| Attachment: | LocMemlocking.diff added |
|---|
comment:1 by , 12 years ago
| Component: | Uncategorized → Core (Cache system) |
|---|---|
| Patch needs improvement: | set |
| Triage Stage: | Unreviewed → Accepted |
| Type: | Uncategorized → Bug |
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 , 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.
comment:3 by , 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 , 12 years ago
| Needs documentation: | set |
|---|
Testcode and your patch needs some inline comments
comment:5 by , 12 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 , 11 years ago
| Resolution: | → duplicate |
|---|---|
| Status: | new → closed |
Duplicate of #20613, I'll merge the test that's included in this PR though as tests weren't added with that ticket.
LocMemlocking.diff