LRUCache can permit data races by accepting non-Sendable Value types

[mattmassicotte]

I'm experimenting with LRUCache and it's super cool! However, I noticed that it is a Sendable container type with generic params that are not all themselves Sendable. And indeed, I have found an issue. Here's a reproduction:

import LRUCache

class NonSendable {
}

@MainActor
func unsafeExample() {
    let cache = LRUCache<String, NonSendable>()
    let ns = NonSendable()

    cache.setValue(ns, forKey: "bad")

    Task.detached {
        let smuggledValue = cache.value(forKey: "bad")

        print("accessing in the background:", smuggledValue as Any)
    }

    print("accessing on the main thread:", ns as Any)
}

The most straightforward fix for this is to require that LRUCache.Value be Sendable. However, this is API-breaking and I'm not sure how big an issue that is.

Another, much more minor problem I noticed is with the atomic helper.

func atomic<T>(_ action: () -> T) -> T {
    lock.lock()
    defer { lock.unlock() }
    return action()
}

Generally speaking, this is also a tool you can use to smuggle values across actor boundaries. In 6.0+, you can use sending to address this, but to maintain compatibility with < 6.0, you must use Sendable here too. I don't imagine that will be a huge problem though given the rest of the implementation.

[nicklockwood]

Thanks for reporting, Matt!

Yeah, I think making the generic type Sendable is the way to go here. The breaking change is annoying, but seems unavoidable

[lhoward]

Jumping in, it would be (for Swift 6) nice to replace NSLock with Mutex.

Edit: this is actually non-trivial because you need to protect head and tail as well. May not be worth it.