Follow up of the fix making jar file reference close idempotent with …

…minor comments and refactor

independent-projects/bootstrap/runner/src/main/java/io/quarkus/bootstrap/runner/JarFileReference.java

@@ -15,6 +15,7 @@ public class JarFileReference {
 
  // This is required to perform cleanup of JarResource::jarFileReference without breaking racy updates
  private CompletableFuture<JarFileReference> completedFuture;
+
  // Guarded by an atomic reader counter that emulate the behaviour of a read/write lock.
  // To enable virtual threads compatibility and avoid pinning it is not possible to use an explicit read/write lock
  // because the jarFile access may happen inside a native call (for example triggered by the RunnerClassLoader)
@@ -30,20 +31,6 @@ private JarFileReference(JarFile jarFile) {
  this.jarFile = jarFile;
  }
 
- public static JarFileReference completeWith(CompletableFuture<JarFileReference> completableFuture, JarFile jarFile) {
- Objects.requireNonNull(completableFuture);
- var jarFileRef = new JarFileReference(jarFile);
- jarFileRef.completedFuture = completableFuture;
- completableFuture.complete(jarFileRef);
- return jarFileRef;
- }
-
- public static CompletableFuture<JarFileReference> completedWith(JarFile jarFile) {
- var jarFileRef = new JarFileReference(jarFile);
- jarFileRef.completedFuture = CompletableFuture.completedFuture(jarFileRef);
- return jarFileRef.completedFuture;
- }
-
  /**
  * Increase the readers counter of the jarFile.
  *
@@ -57,21 +44,20 @@ private boolean acquire() {
  if (count == 0) {
  return false;
  }
- if (referenceCounter.compareAndSet(count, addCount(count, 1))) {
+ if (referenceCounter.compareAndSet(count, changeReferenceCount(count, 1))) {
  return true;
  }
  }
  }
 
  /**
- * This is not allowed to change the sign of count (unless put it to 0)
+ * Change the absolute value of the provided reference count of the given delta, that can only be 1 when the reference is
+ * acquired by a new reader or -1 when the reader releases the reference or the reference itself is marked for closing.
+ * A negative reference count means that this reference has been marked for closing.
  */
- private static int addCount(final int count, int delta) {
+ private static int changeReferenceCount(final int count, int delta) {
  assert count != 0;
- if (count < 0) {
- delta = -delta;
- }
- return count + delta;
+ return count < 0 ? count - delta : count + delta;
  }
 
  /**
@@ -89,17 +75,18 @@ private boolean release(JarResource jarResource) {
  if (count == 1 || count == 0) {
  throw new IllegalStateException("Duplicate release? The reference counter cannot be " + count);
  }
- if (referenceCounter.compareAndSet(count, addCount(count, -1))) {
+ if (referenceCounter.compareAndSet(count, changeReferenceCount(count, -1))) {
  if (count == -1) {
- silentCloseJarResources(jarResource);
+ // The reference has been already marked to be closed (the counter is negative) and this is the last reader releasing it
+ closeJarResources(jarResource);
  return true;
  }
  return false;
  }
  }
  }
 
- private void silentCloseJarResources(JarResource jarResource) {
+ private void closeJarResources(JarResource jarResource) {
  // we need to make sure we're not deleting others state
  jarResource.jarFileReference.compareAndSet(completedFuture, null);
  try {
@@ -110,7 +97,7 @@ private void silentCloseJarResources(JarResource jarResource) {
  }
 
  /**
- * Ask to close this reference.
+ * Mark this jar reference as ready to be closed.
  * If there are no readers currently accessing the jarFile also close it, otherwise defer the closing when the last reader
  * will leave.
  */
@@ -122,9 +109,10 @@ void markForClosing(JarResource jarResource) {
  return;
  }
  // close must change the value into a negative one or zeroing
- if (referenceCounter.compareAndSet(count, addCount(-count, -1))) {
+ // the reference counter is turned into a negative value to indicate (in an idempotent way) that the resource has been marked to be closed.
+ if (referenceCounter.compareAndSet(count, changeReferenceCount(-count, -1))) {
  if (count == 1) {
- silentCloseJarResources(jarResource);
+ closeJarResources(jarResource);
  }
  }
  }
@@ -145,6 +133,7 @@ static <T> T withJarFile(JarResource jarResource, String resource, JarFileConsum
  if (jarFileReference.acquire()) {
  return consumeSharedJarFile(jarFileReference, jarResource, resource, fileConsumer);
  }
+ // The acquire failure implies that the reference is already marked to be closed.
  closingLocalJarFileRef = true;
  }
 
@@ -205,6 +194,12 @@ private static CompletableFuture<JarFileReference> syncLoadAcquiredJarFile(JarRe
  }
  }
 
+ private static CompletableFuture<JarFileReference> completedWith(JarFile jarFile) {
+ var jarFileRef = new JarFileReference(jarFile);
+ jarFileRef.completedFuture = CompletableFuture.completedFuture(jarFileRef);
+ return jarFileRef.completedFuture;
+ }
+
  private static JarFileReference asyncLoadAcquiredJarFile(JarResource jarResource) {
  CompletableFuture<JarFileReference> newJarRefFuture = new CompletableFuture<>();
  CompletableFuture<JarFileReference> existingJarRefFuture = null;
@@ -223,4 +218,12 @@ private static JarFileReference asyncLoadAcquiredJarFile(JarResource jarResource
  } while (existingJarRef == null || !existingJarRef.acquire());
  return existingJarRef;
  }
+
+ private static JarFileReference completeWith(CompletableFuture<JarFileReference> completableFuture, JarFile jarFile) {
+ Objects.requireNonNull(completableFuture);
+ var jarFileRef = new JarFileReference(jarFile);
+ jarFileRef.completedFuture = completableFuture;
+ completableFuture.complete(jarFileRef);
+ return jarFileRef;
+ }
 }