Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Authentication Cache + Storage crash application #3166

Closed
MFranceschi6 opened this issue Apr 3, 2024 · 2 comments
Closed

Authentication Cache + Storage crash application #3166

MFranceschi6 opened this issue Apr 3, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@MFranceschi6
Copy link

Describe the issue

Application crash because of a req.auth.require(Something.self)

Vapor version

4.84.2

Operating system and version

macos 14.4.1 (23E224) apple m2

Swift version

Swift Package Manager - Swift 5.10.0-dev

Steps to reproduce

create a simple Vapor application and use this as handle and build in release mode.

struct Test: Authenticatable { }

func test(req: Request) async throws -> Response {
    await withThrowingTaskGroup(of: Void.self)  { group in
	for i in (0...5) {
	    group.addTask {
		try? req.auth.require(Test.self)
	    }
	}
    }

    return .init()
}

call the handle, usually around the second call it crashes.

I've dug into the code and I've found that the problem is the following pattern inside a concurrently executing code

if let existing = req.storage.get(Test.self) {
    req.logger.info("get")
} else {
    req.storage[Test.self] = .init()
    req.logger.info("Set")
}

This is exactly what we find here

private var cache: Cache {
get {
if let existing = self.request.storage[CacheKey.self] {
return existing
} else {
let new = Cache()
self.request.storage[CacheKey.self] = new
return new
}
}
set {
self.request.storage[CacheKey.self] = newValue
}
}

Something like this fixes the issue, but I suppose it's too simple of a solution and also I don't know the performances impact it may have.

let lock: NIOLock = .init()
lock.withLock {
	if let existing = req.storage.get(Test.self) {
		req.logger.info("get")
	} else {
		req.storage[Test.self] = .init()
		req.logger.info("Set")
	}
}

Outcome

The program goes boom 😄

Additional notes

The same issue arises in an amazon linux 2 container with swift 5.7

@MFranceschi6 MFranceschi6 added the bug Something isn't working label Apr 3, 2024
@0xTim
Copy link
Member

0xTim commented Apr 3, 2024

@MFranceschi6 have you tried the latest version of Vapor? These should all be fixed in 4.86.0

@MFranceschi6
Copy link
Author

@0xTim thank you very much, I also did a swift package update before testing the bug.
I don't know why it didn't update the version of vapor.

Yep, now I've 4.92.5 and it works flawlessly thank you again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants