[Blocked] Exclusive Cache Lock

android-test/src/androidTest/java/okhttp/android/test/OkHttpTest.kt

@@ -859,6 +859,7 @@ class OkHttpTest {
     assertEquals(setOf(OkHttpTest::class.java.name), testHandler.calls.keys)
   }
 
+  @Test
   fun testCachedRequest() {
     enableTls()
 

okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsTest.kt

@@ -170,7 +170,7 @@ class DnsOverHttpsTest {
   // 5. unsuccessful response
   @Test
   fun usesCache() {
-    val cache = Cache("cache".toPath(), (100 * 1024).toLong(), cacheFs)
+    val cache = Cache("cache-usesCache".toPath(), (100 * 1024).toLong(), cacheFs)
     val cachedClient = bootstrapClient.newBuilder().cache(cache).build()
     val cachedDns = buildLocalhost(cachedClient, false)
 
@@ -208,7 +208,7 @@ class DnsOverHttpsTest {
 
   @Test
   fun usesCacheEvenForPost() {
-    val cache = Cache("cache".toPath(), (100 * 1024).toLong(), cacheFs)
+    val cache = Cache("cache-usesCacheEvenForPost".toPath(), (100 * 1024).toLong(), cacheFs)
     val cachedClient = bootstrapClient.newBuilder().cache(cache).build()
     val cachedDns = buildLocalhost(cachedClient, false, post = true)
     repeat(2) {
@@ -241,6 +241,8 @@ class DnsOverHttpsTest {
     assertThat(recordedRequest.method).isEqualTo("POST")
     assertThat(recordedRequest.path)
       .isEqualTo("/lookup?ct")
+
+    cache.close()
   }
 
   @Test
@@ -282,6 +284,8 @@ class DnsOverHttpsTest {
     assertThat(recordedRequest!!.method).isEqualTo("GET")
     assertThat(recordedRequest.path)
       .isEqualTo("/lookup?ct&dns=AAABAAABAAAAAAAABmdvb2dsZQNjb20AAAEAAQ")
+
+    cache.close()
   }
 
   private fun dnsResponse(s: String): MockResponse {

okhttp/src/main/kotlin/okhttp3/internal/cache/CacheLock.kt

@@ -0,0 +1,81 @@
+/*
+ * Copyright (C) 2024 Block, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package okhttp3.internal.cache
+
+import android.annotation.SuppressLint
+import java.io.Closeable
+import java.nio.channels.FileChannel
+import java.nio.file.StandardOpenOption
+import java.util.Collections
+import okhttp3.internal.platform.Platform
+import okio.FileSystem
+import okio.Path
+import org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement
+
+internal object CacheLock {
+  private val openCaches = Collections.synchronizedMap(mutableMapOf<Path, Exception>())
+
+  fun openLock(
+    fileSystem: FileSystem,
+    directory: Path,
+  ): Closeable {
+    return if (!Platform.isAndroid) {
+      fileSystemLock(inMemoryLock(directory), directory)
+    } else {
+      inMemoryLock(directory)
+    }
+  }
+
+  /**
+   * Create an in-memory lock, avoiding two open Cache instances.
+   */
+  @SuppressLint("NewApi")
+  @IgnoreJRERequirement // D8 supports put if absent
+  fun inMemoryLock(directory: Path): Closeable {
+    val existing = openCaches.putIfAbsent(directory, Exception("CacheLock($directory)"))
+    if (existing != null) {
+      throw IllegalStateException("Cache already open at '$directory' in same process", existing)
+    }
+    return okio.Closeable {
+      openCaches.remove(directory)
+    }
+  }
+
+  /**
+   * Create a file system lock, that excludes other processes. However within the process a
+   * memory lock is also needed, since locks don't work within a single process.
+   */
+  @SuppressLint("NewApi")
+  @IgnoreJRERequirement // only called on JVM
+  fun fileSystemLock(
+    memoryLock: Closeable,
+    directory: Path,
+  ): Closeable {
+    val lockFile = directory / "lock"
+    println("Locking $lockFile")
+    lockFile.toFile().createNewFile()
+    val channel = FileChannel.open(lockFile.toNioPath(), StandardOpenOption.APPEND)
+
+    checkNotNull(channel.tryLock()) {
+      "Cache already open at '$directory' in another process"
+    }
+
+    return okio.Closeable {
+      memoryLock.close()
+      channel.close()
+    }
+  }
+}

okhttp/src/main/kotlin/okhttp3/internal/cache/DiskLruCache.kt

@@ -20,6 +20,7 @@ import java.io.EOFException
 import java.io.Flushable
 import java.io.IOException
 import okhttp3.internal.assertThreadHoldsLock
+import okhttp3.internal.cache.CacheLock.openLock
 import okhttp3.internal.cache.DiskLruCache.Editor
 import okhttp3.internal.closeQuietly
 import okhttp3.internal.concurrent.Task
@@ -95,6 +96,8 @@ class DiskLruCache(
   /** Used for asynchronous journal rebuilds. */
   taskRunner: TaskRunner,
 ) : Closeable, Flushable {
+  lateinit var cacheLock: Closeable
+
   internal val fileSystem: FileSystem =
     object : ForwardingFileSystem(fileSystem) {
       override fun sink(
@@ -242,6 +245,8 @@ class DiskLruCache(
 
     civilizedFileSystem = fileSystem.isCivilized(journalFileBackup)
 
+    cacheLock = openLock(fileSystem, directory)
+
     // Prefer to pick up where we left off.
     if (fileSystem.exists(journalFile)) {
       try {
@@ -705,6 +710,8 @@ class DiskLruCache(
       return
     }
 
+    cacheLock.close()
+
     // Copying for concurrent iteration.
     for (entry in lruEntries.values.toTypedArray()) {
       if (entry.currentEditor != null) {

okhttp/src/test/java/okhttp3/CacheLockTest.kt

@@ -0,0 +1,126 @@
+/*
+ * Copyright (C) 2024 Block, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package okhttp3
+
+import assertk.assertThat
+import assertk.assertions.isEqualTo
+import java.nio.file.Path
+import okhttp3.testing.PlatformRule
+import okhttp3.testing.PlatformVersion
+import okio.Closeable
+import okio.FileSystem
+import okio.Path.Companion.toOkioPath
+import okio.Path.Companion.toPath
+import org.junit.jupiter.api.AfterEach
+import org.junit.jupiter.api.BeforeEach
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.assertThrows
+import org.junit.jupiter.api.extension.RegisterExtension
+import org.junit.jupiter.api.io.TempDir
+
+@org.junit.jupiter.api.parallel.Isolated
+class CacheLockTest {
+  @JvmField
+  @RegisterExtension
+  val platform = PlatformRule()
+
+  private lateinit var tempDir: okio.Path
+  private val toClose = mutableListOf<Closeable>()
+
+  @BeforeEach
+  fun setup(
+    @TempDir tempDir: Path,
+  ) {
+    this.tempDir = tempDir.toOkioPath()
+  }
+
+  @AfterEach
+  fun cleanup() {
+    toClose.forEach {
+      it.close()
+    }
+  }
+
+  @Test
+  fun testCacheLock() {
+    openCache(tempDir)
+
+    val ioe =
+      assertThrows<IllegalStateException> {
+        openCache(tempDir)
+      }
+    assertThat(ioe.message).isEqualTo("Cache already open at '$tempDir' in same process")
+  }
+
+  @Test
+  fun testCacheLockAfterClose() {
+    val cache1 = openCache(tempDir)
+
+    cache1.close()
+
+    openCache(tempDir)
+  }
+
+  @Test
+  fun testCacheLockDifferentPath() {
+    openCache(tempDir / "a")
+
+    openCache(tempDir / "b")
+  }
+
+  @Test
+  fun testCacheLockDifferentProcess() {
+    val lockFile = tempDir / "lock"
+    lockFile.toFile().createNewFile()
+
+    val javaExe = if (PlatformVersion.majorVersion >= 9) {
+      @Suppress("Since15")
+      ProcessHandle.current().info().command().get().toPath()
+    } else {
+      System.getenv("JAVA_HOME").toPath() / "bin/java"
+    }
+
+    assertThat(FileSystem.SYSTEM.exists(javaExe))
+
+    val process =
+      ProcessBuilder().command(javaExe.toString(), "src/test/java/okhttp3/LockTestProgram.java", (lockFile.toString()))
+        .redirectErrorStream(true)
+        .start()
+
+    val output = process.inputStream.bufferedReader()
+
+    try {
+      assertThat(output.readLine()).isEqualTo("Locking $lockFile")
+      assertThat(output.readLine()).isEqualTo("Locked $lockFile")
+
+      val ioe =
+        assertThrows<IllegalStateException> {
+          openCache(tempDir)
+        }
+      assertThat(ioe.message).isEqualTo("Cache already open at '$tempDir' in another process")
+    } finally {
+      process.destroy()
+    }
+  }
+
+  private fun openCache(directory: okio.Path): Cache {
+    return Cache(directory, 10_000, FileSystem.SYSTEM).apply {
+      // force early LRU initialisation
+      initialize()
+      toClose.add(this)
+    }
+  }
+}

okhttp/src/test/java/okhttp3/LockTestProgram.java

@@ -0,0 +1,22 @@
+package okhttp3;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.channels.FileChannel;
+import java.nio.file.StandardOpenOption;
+
+public class LockTestProgram {
+  public static void main(String[] args) throws IOException {
+    File lockFile = new File(args[0]);
+
+    System.out.println("Locking " + lockFile);
+
+    FileChannel channel = FileChannel.open(lockFile.toPath(), StandardOpenOption.APPEND);
+
+    channel.lock();
+
+    System.out.println("Locked " + lockFile);
+
+    System.in.read();
+  }
+}