unavoidable race condition in URLCache

Also, what do they mean by "the same request"?

The cache implements HTTP semantics, so that’s where you should look for the definition of “same”.

Keep in mind that HTTP cache semantics are very flexible. There are plenty of situations where you might expect the cache to cache things but it doesn’t, and that’s allowed by the spec.

If I put a lock … in my wrappers on top of "cachedResponse" / "storeCachedResponse" would that avoid the mentioned race condition?

Given its size and complexity, adding a lock around URLCache calls is just begging for a deadlock.

What does it mean "unavoidable"?

The cache has no concept of reserved entries. Imagine two threads, A and B, that are fetching the same request. You might see this sequence:

1. Thread A checks the cache.

2. It’s not there, so A starts request A and waits for the response.

3. Thread B checks the cache.

4. It’s not there, so B starts request B and waits for the response.

5. Request A finished, resulting in response A. Thread A commits to that to the cache.

6. Request B finished, resulting in response B. What does thread B do?

There are two issues here:

• Both requests A and B are run, which is wasteful.

• If responses A and B are different, different values land in the cached depending on the order of steps 5 and 6 and B’s policy.

It’s common for caches to solve this by implementing reservations — that is, the spot for response A is reserved in the cache, thread B learns about that, and waits on that — but the URLCache API does not support this.

You could fix this by adding your cache control on top of URLCache but:

• It only works if you control all the cache’s clients. If, for example, you’re using URLSession, you don’t have that control.

• Once you start down this path, you have to ask yourself whether it’s worthwhile using URLCache at all. What is it buying you?

Share and Enjoy
—
Quinn “The Eskimo!” @ Developer Technical Support @ Apple
let myEmail = "eskimo" + "1" + "@" + "apple.com"

I use URLCache to not implement caching myself; it does everything I need out of the box, through I admit my caching needs are pretty basic: essentially a key / data store with ability to specify total size, with persistence & encryption, cleaned up by OS either partially or completely if OS needs disk or RAM. If I were to implement caching myself (which I can) that would be quite some chunk of work I'd rather not do, testing and handling edge cases (like sanitising and deleting cache after app crash that left cache in an inconsistent state). Here I follow the "Best code is no code" strategy (or, like in this case the well tested code that is maintained by the system itself).

Your example with "waste" + "override" is understandable, although not a show stopper. If I read the warning sentence above properly (which I am not sure I do!) that "concurrent read/write calls themselves to the same resource can crash" (similar to how unprotected read / modify of a dictionary can crash), then this is what I plan doing:

extension URLCache {

    static let lock = NSLock()

    func storeCachedResponse_mt(_ response: CachedURLResponse, for request: URLRequest) {
        Self.lock.lock()
        defer { Self.lock.unlock() }
        storeCachedResponse(response, for: request)
    }

    func cachedResponse_mt(for request: URLRequest) {
        Self.lock.lock()
        defer { Self.lock.unlock() }
        return cachedResponse(for: request)
    }

    func removeCachedResponse_mt(for request: URLRequest) {
        Self.lock.lock()
        defer { Self.lock.unlock() }
        removeCachedResponse(for: request)
    }
}

Does it make sense? Under what circumstances can this deadlock?

I also found some strange behaviour in a single threaded scenario:

    removeCachedResponse(for: request)
    cachedResponse(for: request) // still returns data sometimes!

This is a single threaded case when no-one else is writing the cache. ditto for "set + get" with get sometimes returning the old value. It feels like "remove"/"set" is somewhat asynchronous.. There is nothing in the docs abut this behaviour. I put some workaround but I am not totally happy about it.

Cache reservation you are talking about is probably doable via (pseudocode):

func read() {
    lock()
    result = get()
    if result == nil {
        set(loadingMarker)
        unlock()
        loadResource() { data, error in
            if not error {
                lock()
                set(data)
                unlock()
            } else {
                lock()
                set(nil) // or set(error)
                unlock()
            }
        }
    }
    unlock()
}

where loading / error markers can be stored as a userInfo attribute of a cached entry.

If I read the warning sentence above properly … that "concurrent read/write calls themselves to the same resource can crash"

That’s an incorrect reading of that sentence. URLCache is already thread safe, for that definition thread safe. The docs are warning you that thread safety is not the only race condition you have to worry about.

Does it make sense?

No. That doesn’t buy you any thread safety above and beyond what URLCache already supports.

I also found some strange behaviour in a single threaded scenario

Yep, I’ve seen that myself. I believe it comes about because URLCache defers write operations on a background queue and so your subsequente read operation races with that.

Again, keep in mind the big picture here: URLCache was designed to meet the needs of an HTTP client, Safari basically, not as a general-purpose caching API.

Cache reservation you are talking about is probably doable

It’s certainly doable but that code is not sufficient. To make this work you need a primitive other than a lock because other threads have to be able to block on the reservation waiting for the ‘lead thread’ to finish the load.

Share and Enjoy
—
Quinn “The Eskimo!” @ Developer Technical Support @ Apple
let myEmail = "eskimo" + "1" + "@" + "apple.com"

No. That doesn’t buy you any thread safety above and beyond what URLCache already supports.

Got you, will not bother with the locks then.

Yep, I’ve seen that myself. I believe it comes about because URLCache defers write operations on a background queue and so your subsequente read operation races with that.

I suspected this. A better behaviour would be: update memory representation, return updated memory representation for readers and if needed do the writing on a background queue (but that should not affect readers).

Again, keep in mind the big picture here: URLCache was designed to meet the needs of an HTTP client, Safari basically, not as a general-purpose caching API.

I hear you. There are pros and cons of course.

It’s certainly doable but that code is not sufficient. To make this work you need a primitive other than a lock because other threads have to be able to block on the reservation waiting for the ‘lead thread’ to finish the load.

I generally hate locking, especially long term... IMHO - that's the recipe for deadlocks... I would make other clients getting some placeholder data (e.g. "loading" image if that's an image) without blocking. Once the data is finally loaded by the lead thread - the corresponding model state is changed and the relevant updates should be sent so that the all interesting parties update their representations (e.g. via SwiftUI's observation machinery, etc).