Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#20613 closed Bug (fixed)

LocMem cache may cause deadlocks

Reported by: Rafal Stozek 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


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

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

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

comment:3 Changed 3 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 3 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 3 years ago by anonymous

Patch needs improvement: unset
Version: 1.5master

comment:6 Changed 3 years ago by anonymous

Resolution: fixed
Status: newclosed

comment:7 Changed 3 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