Skip to content

Run and Fix CacheTest emulating windows - #9339

Closed
yschimke wants to merge 8 commits into
lysine-dev:mainfrom
yschimke:fix_windows_caching
Closed

yschimke wants to merge 8 commits into
lysine-dev:mainfrom
yschimke:fix_windows_caching

Conversation

@yschimke

Copy link
Copy Markdown
Collaborator

Investigate and fix issues affecting windows.
From #9335

Investigate and fix issues affecting windows.
From lysine-dev#9335
Investigate and fix issues affecting windows.
From lysine-dev#9335
…windows_caching

# Conflicts:
#	okhttp/src/jvmTest/kotlin/okhttp3/CacheTest.kt
@yschimke

yschimke commented Feb 22, 2026

Copy link
Copy Markdown
Collaborator Author

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
Copy Markdown
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
@yschimke yschimke closed this Aug 3, 2026
@Nohus

Nohus commented Aug 3, 2026

Copy link
Copy Markdown

@yschimke Hello, I was following this PR with much hope as I raised the original issue. Why was it closed?

@yschimke

yschimke commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

I thought there was one further along, non draft. Let me check and reopen if not

@yschimke yschimke reopened this Aug 3, 2026
@yschimke

Copy link
Copy Markdown
Collaborator Author

closing in favour of #9446

@yschimke yschimke closed this Sep 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants