Lock reftable auto-refresh to ensure consistencystable-7.2

Ensure that reftable auto-refresh operations, clearing the database
cache and reloading the reftable stack are executed in an exclusive
critical section under lock. Previously, these steps were performed
without an exclusive critical section, creating a window where
concurrent threads could interfere with each other.

In a race condition, one thread might clear the cache and before it had
a chance of reloading the stack, another thread could repopulate the
cache with stale data, keeping a reference to the open BlockSource
channel to the underlying tables that are subsequently removed when the
first thread reloads the stack.

The above race condition resulted in attempts to access closed resources
and lead to ClosedChannelException errors.

As an example, consider the following scenario:

* T0 - Thread-1 is executing auto-refresh and it clears the database
       cache

* T1 - The master branch moves forward (for any reason):
         - A new refTable (`R_new`) file is created
         - An existing refTable (`R_old`) file is deleted due to
           auto-compaction.

* T3 - Thread-2 repopulates the database cache before Thread-1 has had a
       chance to reload the refTable stack.

* T4 - Thread-1 finally reloads the refTable stack, causing the closing
       of the BlockSource wrapping the removed `R_old` refTable file.

* T5 - Thread-2 attempts to read from the already-closed `R_old`
       BlockSource and the `j.n.c.ClosedChannelException` is thrown

To reproduce this problem, you can run a script created to craft this
racing condition: I1e78e175cff.

While such errors during concurrent execution might be expected and
tolerable in isolation, the situation becomes more severe when the
`RepositoryCache` is involved, as is the case with Gerrit.

The `FileReftableDatabase` instance is cached within the `Repository`
object. When a `BlockSource` is closed prematurely due to this race
condition, the dangling reference remains in memory until the cached
Repository expires, which is one hour by default.

This means that, once the race occurs, the repository may be unable to
perform any ref lookups for up to an hour, effectively causing a
repository outage.

By introducing a ReentrantLock around both operations, the refresh logic
now guarantees that concurrent readers and writers maintain a consistent
view of the reftable state, eliminating the race condition.

Verified by executing the script provided at I1e78e175cff and no
exceptions are raised anymore.

Bug: jgit-130
Change-Id: I6153528a7b2695115b670bda04d4d4228c1731e1

1 files changed, 8 insertions, 2 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java
index e9782e2e18..d5a060f06c 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java
@@ -270,8 +270,14 @@ public class FileReftableDatabase extends RefDatabase {
 	public void refresh() {
 		try {
 			if (!reftableStack.isUpToDate()) {
-				reftableDatabase.clearCache();
-				reftableStack.reload();
+				ReentrantLock lock = getLock();
+				lock.lock();
+				try {
+					reftableDatabase.clearCache();
+					reftableStack.reload();
+				} finally {
+					lock.unlock();
+				}
 			}
 		} catch (IOException e) {
 			throw new UncheckedIOException(e);