*: Bump async-std to v1.6.5 #7306
Conversation
Prevent users from using v1.6.4 which faces issues receiving incoming TCP connections. See async-rs/async-std#888 for details.
mxinden
added
A0-please_review
|
As we had seen last week in polkadot, this makes the web wasm build fail. |
`async_std::sync::Condvar::wait_timeout` uses `gloo_timers::callback::Timeout` when compiled for `wasm32-unknown-unknown`. This timeout implementation does not fulfill the requirement of being `Send`. Instead of using a `Condvar` use a `futures::channel::mpsc` to signal progress from the `QueuedSender` to the background `Future`.
This would be fixed with 96c9f6b which uses a channel instead of a condvar as suggested here: See rustwasm/gloo#109 for details on Before I iron out any outstanding |
tomaka
added
the
A1-needs_burnin
There was a problem hiding this comment.
tomaka added the A1-needsburnin label 43 minutes ago
I don't think the Guess it is still worth testin async-std 1.6.5 itself.needsburnin label is helpful here. As far as I can tell QueuedSender is never constructed within the Substrate or Polkadot codebase apart from unit tests.
client/network/src/gossip.rs
Outdated
| @@ -67,6 +67,9 @@ mod tests; | |||
| pub struct QueuedSender<M> { | |||
| /// Shared between the front and the back task. | |||
| shared: Arc<Shared<M>>, | |||
| // TODO: Can we get around this Mutex? E.g. by taking `&mut` for `lock_queue` and | |||
| // `queue_or_discard`. | |||
| notify_background_future: Mutex<Sender<()>>, | |||
There was a problem hiding this comment.
@tomaka I would like to remove the need for this Mutex, thus requiring QueuedSender::lock_queue and QueuedSender::queue_or_discard to take a &mut self. Is the current &self a design requirement?
There was a problem hiding this comment.
Good question. I think you can switch to &mut self, and we can always switch back later if needed.
The `queue_size_limit` field is only accessed by `QueuedSender`, thus there is no need to share it between the background future and the `QueuedSender`.
To be a bit picky the background task is not a task in the sense of an asynchonous task, but rather a background future in the sense of `futures::future::Future`.
mxinden
added
B3-apinoteworthy
and removed
B0-silent
|
@tomaka would you mind taking another look? |
mxinden
added
A1-needs_burnin
|
This patch has been deployed on two sentry nodes for the last 3 hours. Substrate service tasks, Substrate networking nor the Node Exporter dashboard show any anomalies. Only thing sticking out is the number of peers the nodes have more than one connection to. This seems to be increasing since the redeployment of the nodes:
Update: Stabilized in the last couple of days. |
|
bot merge |
|
Trying merge. |
ghost
merged commit 
Prevent users from using v1.6.4 which faces issues receiving incoming
TCP connections. See async-rs/async-std#888
for details.
//CC @JoshOrndorff
Release note suggestion: