-
Notifications
You must be signed in to change notification settings - Fork 78
Replace parking_lot Mutex with tokio Mutex to align with kona-node async primitives #433
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
| use http::Uri; | ||
| use jsonrpsee::core::client::ClientT; | ||
| use parking_lot::Mutex; | ||
| use std::sync::Mutex; |
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.
nit: We should tokio mutexes here.
| use std::sync::Mutex; | |
| use tokio::sync::Mutex; |
| use std::net::TcpListener; | ||
| use std::path::PathBuf; | ||
| use std::str::FromStr; | ||
| use std::sync::Mutex; |
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.
We should use tokio mutexes here
| use std::sync::Mutex; | |
| use tokio::sync::Mutex; |
0xForerunner
left a comment
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.
parking_lot::Mutex is incompatible with certain concurrency primitives used by kona-node.
I'm not convinced that this is necessary. From tokios own documentation:
Contrary to popular belief, it is ok and often preferred to use the ordinary Mutex from the standard library in asynchronous code.
Maybe you could provide some more detailed information on why you think this is required.
Motivation
Change summary
lock().awaitand adjust signatures to be async where necessary.std::sync::Mutex.Scope
server,health,probe,debug_api(runtime paths) now usetokio::sync::Mutex.std::sync::Mutex..awaitinside non-async contexts.Incidental behavior adjustments (in service of the migration)
Testing and verification
std::sync::Mutexfor mock state.Risk and rollout
Follow-ups
flashblocks/inbound.rs) for any lingeringparking_lot::Mutexon async paths and migrate totokio::sync::Mutexwhere applicable.