commit df56399d784094a24c0ab784c3eddee0908952eb
author Max Kellermann <mk@cm4all.com> Fri Mar 05 21:27:10 2021 +0100
committer GitHub <noreply@github.com> Fri Mar 05 15:27:10 2021 -0500
tree b9b9d0aa66ed5dc329d436272ed8fe6cb7c90cdd
parent 7b7f61c3b99f0bb9827ad4c361421f8df1d9591b

Rework SoftRefFilesCache locking (#154)

* SoftRefFilesCache: no ReferenceQueue timeout, sleep indefinitely

Waking up once a second is just a waste of CPU time.  Let
ReferenceQueue.remove() only wake up if there's something to do (or if
the thread got interrupted).

* SoftRefFilesCache: eliminate the requestEnd flag

The Thread class keeps track for us already, and if it's interrupted,
we'll catch InterruptedException and quit the thread.

* SoftRefFilesCache: fix ReentrantLock usage in {start,end}Thread()

Can't use the "synchronized" keyword on a Lock; this will only lock
the java.lang.Object, but not the Lock.

This wrong usage was added by commit f09035290b1 (SVN r1704932).

* SoftRefFilesCache: add missing lock to removeFile()

The API documentation of close(FileSystem) requires that the caller
holds the lock, but removeFile() didn't do that.

* SoftRefFilesCache: require lock while calling removeFile(FileSystemAndNameKey)

This allows merging the lock with the caller, fixing a race condition
in removeFile(FileSystem,FileName).

* SoftRefFilesCache: require lock while calling getOrCreateFilesystemCache()

It is an illusion to believe that not requiring the lock would be
faster, because all callers will obtain the lock right after
getOrCreateFilesystemCache() returns.

As a wanted side effect, this also fixes a time-of-check-time-of-use
bug: if the fileSystemCache becomes non-empty by another thread
between the isEmpty() and the get() call (with removeFile() calls by
yet another thread), the SoftRefReleaseThread will never be started.

* SoftRefFilesCache: don't use ConcurrentMap

Now that all accesses to fileSystemCache happen while the lock is
held, we don't need another layer of synchronization.

* SoftRefFilesCache: eliminate "volatile" on softRefReleaseThread

That "double checked locking" in startThread() is rather pointless,
because the overhead saved by not locking never materializes.
startThread() is only ever called when there is no thread, and
optimizing for a corner case with a data race isn't worth the
complexity.  So let's just do everything inside the lock and remove
"volatile".

* SoftRefFilesCache: move endThread() call inside the lock

This avoids locking the Lock twice, because the two lock sections are
merged.

* SoftRefFilesCache: require lock for startThread(), endThread()

All callers already do that, this just documents the fact and removes
the explicit lock()/unlock() calls from those methods.

* SoftRefFilesCache: move code to removeFile(Reference)

* SoftRefFilesCache: eliminate ReentrantLock, use "synchronized"

This class uses no special ReentrantLock/Lock feature, it's just basic
thread synchronization.  Using "synchronized" is easier to use and
less error prone.