Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#20613 closed Bug (fixed)

LocMem cache may cause deadlocks

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

Description

LocMem cache uses locks to synchronize access to the cache dict.

The problem is that pickle dumps/loads is executed inside code blocks which acquired a reader/writer lock. Pickle dumps/loads can execute parts of user's code to accomplish it's task. The problem starts when user's code tries to retrieve something from the cache.

In that situation for example set() acquires a lock, calls pickle.dumps() which calls user's code, which calls get(), which tries to acquire a lock - causing a deadlock. Very rare situation but it does happen.

Change History (7)

comment:1 Changed 10 years ago by Rafal Stozek

The solution would be to move pickle.dumps/loads calls outside of the with self._lock.reader()/writer() blocks.

comment:2 Changed 10 years ago by ersran9

I added a pull request for this. Could someone review it? https://github.com/django/django/pull/1296

comment:3 Changed 10 years ago by Jaap Roes

Has patch: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

Looked at your pull request and found one issue, I've added a line comment on GitHub. Did you run the (cache) tests?

comment:4 Changed 10 years ago by ersran9

I have made the changes that you had suggested. The commit was squashed into the previous pull, not sure if that come out okay. The cache tests does pass.

comment:5 Changed 10 years ago by anonymous

Patch needs improvement: unset
Version: 1.5master

comment:6 Changed 10 years ago by anonymous

Resolution: fixed
Status: newclosed

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

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