Precondition failed in Redis 4.1.0 when using repository pattern

[leonidas-o]

Describe the bug

After updating to 4.1.0 the following error is thrown as soon as I use my cacheRepo.

[ ERROR ] Abort.401: UserModel not authenticated. [request-id: 0661DE6F-B71E-41D6-9B06-2BB4A3F92F9B]
Precondition failed: file .../NIO/ChannelPipeline.swift, line 1447
2021-01-30 15:24:10.165750+0100 Run[13873:207019] Precondition failed: file .../NIO/ChannelPipeline.swift, line 1447

The execution gets stuck at:

    /// Checks the necessary condition of currently running on the called `EventLoop` for making forward progress.
    @inlinable
    public func preconditionInEventLoop(file: StaticString = #file, line: UInt = #line) {
        precondition(self.inEventLoop, file: file, line: line)
    }

Routes without cacheRepo usage, are not affected. Downgrading to redis 4.0.0 shows no errors and everything works as expected.

Inside configure.swift redis is setup using:

    let redisHostname = Environment.get("REDIS_HOSTNAME") ?? "localhost"
    let redisPort: Int
    let redisPassword = Environment.get("REDIS_PASSWORD")
    if (app.environment == .testing) {
        if let testPort = Environment.get("REDIS_PORT_TEST") {
            redisPort = Int(testPort) ?? 6380
        } else {
            redisPort = 6380
        }
    } else {
        redisPort = 6379
    }
    app.redis.configuration = try .init(hostname: redisHostname, port: redisPort, password: redisPassword)

    // specify what repository will be created by cacheRepoFactory
    app.cacheRepoFactory.use { req in
        RedisRepo(client: req.redis)
    }
    app.cacheRepoFactory.useForApp { app in
        RedisRepo(client: app.redis)
    }

RedisRepo contains just the requirements from the cacheRepo protocol:

struct RedisRepo: CacheRepo {
    let client: RedisClient

    // MARK: - JSON Storage

    func save<E>(key: String, to entity: E) -> EventLoopFuture<Void> where E: Encodable {
        let redisKey = RedisKey(key)
        do {
            return client.set(redisKey, to: try JSONEncoder().encode(entity), onCondition: .none, expiration: .keepExisting)
                .transform(to: ())
        } catch {
            return client.eventLoop.makeFailedFuture(error)
        }
    }
    ...

CacheRepoFactory contains:

protocol CacheRepo: ABACCacheRepo {
    func save<E>(key: String, to entity: E) -> EventLoopFuture<Void> where E: Encodable
    func save<E>(key: String, to entity: E, expirationInSeconds seconds: Int) -> EventLoopFuture<Void> where E: Encodable
    func get<D>(key: String, as type: D.Type) -> EventLoopFuture<D?> where D: Decodable
    func getExistingKeys<D>(using keys: [String], as type: [D].Type) -> EventLoopFuture<[D]> where D: Decodable
    func setExpiration(forKey key: String, afterSeconds seconds: Int) -> EventLoopFuture<Bool>
    func delete(key: String) -> EventLoopFuture<Int>
    func delete(keys: [String]) -> EventLoopFuture<Int>
    func timeToLive(key: String) -> EventLoopFuture<Int>
    func exists(_ keys: String...) -> EventLoopFuture<Int>
    func exists(_ keys: [String]) -> EventLoopFuture<Int>
    // hash storage
    func getHash<D>(key: String, field: String, as type: D.Type) -> EventLoopFuture<D?> where D: Decodable
    func getHashAll<D>(key: String, as type: D.Type) -> EventLoopFuture<[String:D]> where D: Decodable
    func setHash<E>(_ key: String, field: String, to entity: E) -> EventLoopFuture<Bool> where E: Encodable
    func setMHash<E>(_ key: String, items: Dictionary<String, E>) -> EventLoopFuture<Void> where E: Encodable
    func deleteHash(_ key: String, fields: String...) -> EventLoopFuture<Int>
    func deleteHash(_ key: String, fields: [String]) -> EventLoopFuture<Int>
}

struct CacheRepoFactory {
    // CacheRepo in Request
    var make: ((Request) -> CacheRepo)?
    mutating func use(_ make: @escaping ((Request) -> CacheRepo)) {
        self.make = make
    }

    // CacheRepo in Application
    var makeForApp: ((Application) -> CacheRepo)?
    mutating func useForApp(_ make: @escaping ((Application) -> CacheRepo)) {
        self.makeForApp = make
    }
}

extension Application {
    private struct CacheRepoKey: StorageKey {
        typealias Value = CacheRepoFactory
    }

    var cacheRepoFactory: CacheRepoFactory {
        get {
            self.storage[CacheRepoKey.self] ?? .init()
        }
        set {
            self.storage[CacheRepoKey.self] = newValue
        }
    }
}

extension Application {
    var cacheRepo: CacheRepo {
        self.cacheRepoFactory.makeForApp!(self)
    }
}

extension Request {
    var cacheRepo: CacheRepo {
        self.application.cacheRepoFactory.make!(self)
    }
}

To Reproduce

First I encountered the error after request.cacheRepo.get(...) was used. But it actually happens on all repo methods. As soon as you try to return request.eventLoop...

struct UserModelBearerAuthenticator: BearerAuthenticator {

    struct AccessDataKey: StorageKey {
        typealias Value = AccessData
    }

    func authenticate(bearer: BearerAuthorization, for request: Request) -> EventLoopFuture<Void> {
        return request.cacheRepo.get(key: bearer.token, as: AccessData.self).flatMap { accessData in
            guard let accessData = accessData else {
                return request.eventLoop.makeSucceededFuture(())
            }
            ...

Steps to reproduce the behavior:

1. Add redis 4.1.0 package
2. Create repository and factory (see code snippets above)
3. Use whatever cacheRepo method you want in a route handler, afterwards every return of a future (req.eventLoop.) will throw an error
4. See error

Expected behavior

No error, same behaviour like in redis 4.0.0.

Environment

• Vapor Framework version: 4.39.2
• Vapor Toolbox version: 18.3.0
• OS version: MacOS Catalina 10.15.7

[pankajsoni19]

Same here. I reverted back to 4.0.0

[Mordil]

Yes, for now our recommendation is to pin Redis as .upToNextMinor(from: "4.0.0") until we can properly get a fix out.

[siemensikkema]

@leonidas-o or @pankajsoni19 could I ask you to try this using: .package(url: "https://gitlab.com/vapor/redis.git", .branch("feature/eventloop-fix"))

[pankajsoni19]

I have moved out of the organization where I wrote the swift based server code. So objective was to attain stability in last deployment under me. I would not be able to give time to test this out.

[leonidas-o]

@siemensikkema I guess you mean github.com/vapor/redis.git and not gitlab...? Okay, that looks good. At least all my routes are working now without throwing an error.

[siemensikkema]

@leonidas-o haha, yeah how did I make that mistake? I blindly copy/pasted and adapted from another line... Great to hear that it's working! That should allow us to avoid reverting 4.1.0.

[leonidas-o]

🙂 never mind and thanks for the fix. I guess this will be just the next tag after 4.1.0?

[0xTim]

@leonidas-o yeah this has been released in 4.1.1

[HashedViking]

I've got a crash on boot when redis version > 4.0.0:

Fatal error: No redis found for id default, or the app may not have finished booting. Also, the eventLoop must be from Application's EventLoopGroup.: file Redis/RedisStorage.swift, line 51

But pinning to 4.0.0 helps: .package(url: "https://github.com/vapor/redis.git", .exact("4.0.0"))

[0xTim]

@HashedViking which version of Redis exactly is causing the crash?

[HashedViking]

@0xTim 4.2.0

boot.swift

public func boot(_ app: Application) throws {   
    let cachedPKsFuture = app.redis.get(RedisKey(FRK.firebasePKs),
                                        asJSON: FirebasePublicKeys.self) // <<--- crash
// ...
}

On boot I try to fetch the data previously cached in Redis, with 4.0.0 this way was ok.

[0xTim]

@HashedViking if you call app.boot() before that does it work?

[HashedViking]

yeah, calling try? app.boot() at the first line fixes the problem, is it intended behaviour?

[0xTim]

Not intentionally but yes it's required because of the way Redis sets up its internal state