tree b9b9d0aa66ed5dc329d436272ed8fe6cb7c90cdd
parent 7b7f61c3b99f0bb9827ad4c361421f8df1d9591b
author Max Kellermann <mk@cm4all.com> 1614976030 +0100
committer GitHub <noreply@github.com> 1614976030 -0500
gpgsig -----BEGIN PGP SIGNATURE-----
 
 wsBcBAABCAAQBQJgQpQeCRBK7hj4Ov3rIwAAdHIIAE3vKFI8zHd8pMN5/M8v2Y12
 lQm4mf/9aosUO9YlV487PvWkoSQBK3RtGyjQt4AapFYD784XhYHh7WW5joJRR9p4
 C+PeXTaYccz9bB/TZxZT+15xVCOihKDwD9yQJ16itXP9GsbwH77c+xjRUn6EWhrb
 Is2Pwqt/Wb1rxTvPEVFsbsoOTHPp9xlasLf4nqitUsIWcpjEQusDtPvQcgY4SDbj
 gHzE4ulZGhvvFHgEFj9mgzriYi/UpGqGtYTrIOPEBcSsP2tHiCv05EC3bqX+uvLh
 OiGbdVx3nGuYCCQfvUJWC24iVgFD6dY7/8O5ihg2sLeBei3HJjITUbg2qhT1HTQ=
 =Bw24
 -----END PGP SIGNATURE-----
 

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.