Skip to content

Run and Fix CacheTest emulating windows#9339

Draft
yschimke wants to merge 7 commits intosquare:masterfrom
yschimke:fix_windows_caching
Draft

Run and Fix CacheTest emulating windows#9339
yschimke wants to merge 7 commits intosquare:masterfrom
yschimke:fix_windows_caching

Conversation

@yschimke
Copy link
Collaborator

Investigate and fix issues affecting windows.
From #9335

Investigate and fix issues affecting windows.
From square#9335
Investigate and fix issues affecting windows.
From square#9335
…windows_caching

# Conflicts:
#	okhttp/src/jvmTest/kotlin/okhttp3/CacheTest.kt
@yschimke
Copy link
Collaborator Author

yschimke commented Feb 22, 2026

The fix isn't trivial. cc @swankjesse

We get here

        // Update the cache after combining headers but before stripping the
        // Content-Encoding header (as performed by initContentStream()).
        cache!!.trackConditionalCacheHit()
        cache.update(cacheResponse, response)

or more precisely

  internal fun update(
    cached: Response,
    network: Response,
  ) {
    val entry = Entry(network)
    val snapshot = (cached.body as CacheResponseBody).snapshot <-- we are still planning to use the cache response body
    var editor: DiskLruCache.Editor? = null
    try {
      editor = snapshot.edit() ?: return // edit() returns null if snapshot is not current.
      entry.writeTo(editor)
      editor.commit()
    } catch (_: IOException) {
      abortQuietly(editor)
    }
  }

we got the cache response body from, so lockingSourceCount is > 0

    private fun newSource(index: Int): Source {
      val fileSource = fileSystem.source(cleanFiles[index])
      if (civilizedFileSystem) return fileSource

      lockingSourceCount++
      return object : ForwardingSource(fileSource) {
        private var closed = false

        override fun close() {
          super.close()
          if (!closed) {
            closed = true
            synchronized(this@DiskLruCache) {
              lockingSourceCount--
              if (lockingSourceCount == 0 && zombie) {
                removeEntry(this@Entry)
              }
            }
          }
        }
      }
    }

So eventually fails here

    if (entry != null && entry.lockingSourceCount != 0) {
      return null // We can't write this file because a reader is still reading it.
    }

@yschimke yschimke changed the title Run CacheTest emulating windows Run and Fix CacheTest emulating windows Feb 22, 2026
@yschimke yschimke marked this pull request as ready for review February 22, 2026 22:38
@yschimke yschimke marked this pull request as draft February 23, 2026 06:42
@yschimke
Copy link
Collaborator Author

At least one of the failures makes it looks expected.

  /**
   * Each read sees a snapshot of the file at the time read was called. This means that two reads of
   * the same key can see different data.
   */
  @Test
  fun readAndWriteOverlapsMaintainConsistency() {
    Assumptions.assumeFalse(windows) // Can't edit while a read is in progress.

@yschimke yschimke requested a review from swankjesse February 23, 2026 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant