Opened 11 years ago

Closed 11 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


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 by Rafal Stozek, 11 years ago

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

comment:2 by ersran9, 11 years ago

I added a pull request for this. Could someone review it?

comment:3 by Jaap Roes, 11 years ago

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 by ersran9, 11 years ago

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 by anonymous, 11 years ago

Patch needs improvement: unset
Version: 1.5master

comment:6 by anonymous, 11 years ago

Resolution: fixed
Status: newclosed

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