Mavenomics / MavenWorks

Agile Dashboarding, anywhere
https://mavenworks.com
GNU General Public License v3.0
18 stars 2 forks source link

Cache.LockManager may drop locks if multiple write locks release at the same time #38

Closed quigleyj-mavenomics closed 5 years ago

quigleyj-mavenomics commented 5 years ago

This is a rather complicated issue in how we've implemented a writer-preferred readers-writer lock in the coreutils Cache.

When a lock releases, a callback in Cache.LockManager removes the lock from the held array and checks to see what locks can be held. If there are no write locks in waiting, then those can all be added at once. Normally (in JS), we can be assured that nothing will change 'underneath' us during the execution of a block if that block is synchronous. For other code paths, this is OK, but in the "no write locks" path of Cache.LockManager#onReleaseLock), we await inside an iterator. This meant that concurrent calls may invalidate this iterator while we were iterating over it- the practical effect of this is that locks may get dropped if multiple locks, one of which is a write lock, release at the same time.

Separately in the future we may want to split off the locking mechanism into it's own class in AsyncTools- readers-writer locks could be useful elsewhere, and it'd help with sorting out some of the logic here when debugging future issues.

Relates to #28, as this was discovered in the process of fixing the MqlCache function to avoid the extra writer locks.