Opened 3 years ago

Closed 3 years ago

Last modified 2 years 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


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

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

comment:3 Changed 3 years 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 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 changed from 1.5 to master

comment:6 Changed 3 years ago by anonymous

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

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