diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/cache/DeferredForwardingSource.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/cache/DeferredForwardingSource.kt new file mode 100644 index 000000000000..750829e813e9 --- /dev/null +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/cache/DeferredForwardingSource.kt @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2025 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 okio.Buffer +import okio.Source + +/** + * Okio ForwardingSource, but without eagerly opening the file. + */ +abstract class DeferredForwardingSource( + delegateFn: () -> Source, +) : Source { + val delegate by lazy(delegateFn) + + override fun read( + sink: Buffer, + byteCount: Long, + ): Long = delegate.read(sink, byteCount) + + override fun timeout() = delegate.timeout() + + override fun close() = delegate.close() + + override fun toString() = "${javaClass.simpleName}($delegate)" +} diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/cache/DiskLruCache.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/cache/DiskLruCache.kt index dbf022e26d19..da134cb55713 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/cache/DiskLruCache.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/cache/DiskLruCache.kt @@ -19,7 +19,6 @@ import java.io.Closeable import java.io.EOFException import java.io.Flushable import java.io.IOException -import okhttp3.internal.cache.DiskLruCache.Editor import okhttp3.internal.closeQuietly import okhttp3.internal.concurrent.Lockable import okhttp3.internal.concurrent.Task @@ -35,7 +34,6 @@ import okio.BufferedSink import okio.FileNotFoundException import okio.FileSystem import okio.ForwardingFileSystem -import okio.ForwardingSource import okio.Path import okio.Sink import okio.Source @@ -1072,21 +1070,29 @@ class DiskLruCache( } private fun newSource(index: Int): Source { - val fileSource = fileSystem.source(cleanFiles[index]) - if (civilizedFileSystem) return fileSource + val file = cleanFiles[index] + if (civilizedFileSystem) return fileSystem.source(file) - lockingSourceCount++ - return object : ForwardingSource(fileSource) { + // Whether the source has been actually acquired + var inited = false + + return object : DeferredForwardingSource({ + inited = true + lockingSourceCount++ + fileSystem.source(file) + }) { private var closed = false override fun close() { super.close() if (!closed) { closed = true - synchronized(this@DiskLruCache) { - lockingSourceCount-- - if (lockingSourceCount == 0 && zombie) { - removeEntry(this@Entry) + if (inited) { + synchronized(this@DiskLruCache) { + lockingSourceCount-- + if (lockingSourceCount == 0 && zombie) { + removeEntry(this@Entry) + } } } } diff --git a/okhttp/src/jvmTest/kotlin/okhttp3/CacheTest.kt b/okhttp/src/jvmTest/kotlin/okhttp3/CacheTest.kt index 573f085b8642..0b75f608fe25 100644 --- a/okhttp/src/jvmTest/kotlin/okhttp3/CacheTest.kt +++ b/okhttp/src/jvmTest/kotlin/okhttp3/CacheTest.kt @@ -15,6 +15,7 @@ */ package okhttp3 +import app.cash.burst.Burst import assertk.assertThat import assertk.assertions.containsExactly import assertk.assertions.isCloseTo @@ -70,7 +71,10 @@ import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.RegisterExtension @Tag("Slow") -class CacheTest { +@Burst +class CacheTest( + val emulatedFileSystem: EmulatedFileSystem = EmulatedFileSystem.Unix, +) { val fileSystem = FakeFileSystem() @RegisterExtension @@ -94,7 +98,11 @@ class CacheTest { fun setUp() { platform.assumeNotOpenJSSE() server.protocolNegotiationEnabled = false - fileSystem.emulateUnix() + if (emulatedFileSystem == EmulatedFileSystem.Windows) { + fileSystem.emulateWindows() + } else { + fileSystem.emulateUnix() + } cache = Cache(fileSystem, "/cache/".toPath(), Long.MAX_VALUE) client = clientTestRule @@ -410,10 +418,13 @@ class CacheTest { assertThat(cache.requestCount()).isEqualTo(2) assertThat(cache.networkCount()).isEqualTo(2) assertThat(cache.hitCount()).isEqualTo(0) + + response1.close() + response2.close() } private fun corruptCertificate(cacheEntry: Path) { - var content = fileSystem.source(cacheEntry).buffer().readUtf8() + var content = fileSystem.source(cacheEntry).buffer().use { it.readUtf8() } content = content.replace("MII", "!!!") fileSystem .sink(cacheEntry) @@ -3788,6 +3799,8 @@ CLEAN $urlKey ${entryMetadata.length} ${entryBody.length} assertThat(response.request.url).isEqualTo(request.url) assertThat(response.cacheResponse!!.request.url).isEqualTo(request.url) + + response.close() } @Test @@ -3807,6 +3820,8 @@ CLEAN $urlKey ${entryMetadata.length} ${entryBody.length} assertThat(response.request.url).isEqualTo(request.url) assertThat(response.cacheResponse!!.request.url).isEqualTo(cacheUrlOverride) + + response.close() } private fun testBasicCachingRules(request: Request): Response { @@ -4025,22 +4040,41 @@ CLEAN $urlKey ${entryMetadata.length} ${entryBody.length} val c = Cache(loggingFileSystem, path, 100000L) assertThat(c.directoryPath).isEqualTo(path) c.size() - assertThat(events).containsExactly( - "metadataOrNull:/cache/journal.bkp", - "metadataOrNull:/cache", - "sink:/cache/journal.bkp", - "delete:/cache/journal.bkp", - "metadataOrNull:/cache/journal", - "metadataOrNull:/cache", - "sink:/cache/journal.tmp", - "metadataOrNull:/cache/journal", - "atomicMove:/cache/journal.tmp", - "atomicMove:/cache/journal", - "appendingSink:/cache/journal", - ) + if (emulatedFileSystem == EmulatedFileSystem.Unix) { + assertThat(events).containsExactly( + "metadataOrNull:/cache/journal.bkp", + "metadataOrNull:/cache", + "sink:/cache/journal.bkp", + "delete:/cache/journal.bkp", + "metadataOrNull:/cache/journal", + "metadataOrNull:/cache", + "sink:/cache/journal.tmp", + "metadataOrNull:/cache/journal", + "atomicMove:/cache/journal.tmp", + "atomicMove:/cache/journal", + "appendingSink:/cache/journal", + ) + } else { + assertThat(events).containsExactly( + "metadataOrNull:/cache/journal.bkp", + "metadataOrNull:/cache", + "sink:/cache/journal.bkp", + "delete:/cache/journal.bkp", + "delete:/cache/journal.bkp", // investigate + "metadataOrNull:/cache/journal", + "metadataOrNull:/cache", + "sink:/cache/journal.tmp", + "metadataOrNull:/cache/journal", + "atomicMove:/cache/journal.tmp", + "atomicMove:/cache/journal", + "appendingSink:/cache/journal", + ) + } events.clear() c.size() assertThat(events).isEmpty() + + c.close() } private fun assertFullyCached(response: MockResponse) { @@ -4126,6 +4160,11 @@ CLEAN $urlKey ${entryMetadata.length} ${entryBody.length} return result } + enum class EmulatedFileSystem { + Unix, + Windows, + } + companion object { private val NULL_HOSTNAME_VERIFIER = HostnameVerifier { hostname, session -> true } }