Opened 22 months ago

Closed 21 months ago

Last modified 14 months ago

#20613 closed Bug (fixed)

LocMem cache may cause deadlocks

Reported by: rafales Owned by: nobody
Component: Core (Cache system) Version: master
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 22 months ago by rafales

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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

comment:2 Changed 21 months ago by ersran9

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

comment:3 Changed 21 months ago by jaap3

  • Has patch set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

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 21 months 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 21 months ago by anonymous

  • Patch needs improvement unset
  • Version changed from 1.5 to master

comment:6 Changed 21 months ago by anonymous

  • Resolution set to fixed
  • Status changed from new to closed

comment:7 Changed 14 months 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