-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add Async Storage shutdown #3196
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A nit and a question, but nothing blocking. I find this incredibly ugly and kinda concerningly fragile, but I have no better suggestions 😕
var onAsyncShutdown: (@Sendable (T) async throws -> ())? | ||
func shutdown(logger: Logger) { | ||
do { | ||
try self.onShutdown?(self.value) | ||
} catch { | ||
logger.warning("Could not shutdown \(T.self): \(error)") | ||
} | ||
} | ||
func asyncShutdown(logger: Logger) async { | ||
do { | ||
try await self.onAsyncShutdown?(self.value) | ||
} catch { | ||
logger.warning("Could not shutdown \(T.self): \(error)") | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a better suggestion, but this seems incredibly error-prone/easy to misuse 😕
public var asyncShared: HTTPClient { | ||
get async { | ||
let lock = self.application.locks.lock(for: Key.self) | ||
lock.lock() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes me uneasy but I don't see a way around it - any thoughts?
Co-authored-by: Gwynne Raskind <gwynne@vapor.codes>
Ok @gwynne I've tested this with client calls as well and think I've fixed all the different areas without breaking the API too much |
These changes are now available in 4.101.0
Currently running
When you try and install NIO as the global executor will crash because the storage API didn't have any async entry points so stopping would trigger a synchronous shutdown, with a
wait()
. This fixes that