Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Add tokio runtime to ethcore io worker #9979

Merged
merged 2 commits into from
Dec 3, 2018
Merged

Conversation

grbIzl
Copy link
Collaborator

@grbIzl grbIzl commented Nov 28, 2018

After #9657 it's assumed, that all threads initialize tokio runtime, if they want to use tokio functionality (like timers). In particular the following scenario was broken:

  • Node receives private transaction packet (network sync module is running in ethcore io worker thread)
  • Private module processes the packet and sends the request to Secret Store in order to decode it
  • Sending the request is done via FetchClient with tokio timeout set for the future
    As ethcore io worker doesn't initialize the runtime, sending the request fails.
    I believe, that this problem can occur not only for private transaction scenario

@grbIzl grbIzl added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Nov 28, 2018
util/io/src/worker.rs Outdated Show resolved Hide resolved
util/io/src/worker.rs Outdated Show resolved Hide resolved
@niklasad1 niklasad1 added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 28, 2018
use std::sync::Arc;
use std::thread::{JoinHandle, self};
use std::sync::atomic::{AtomicBool, Ordering as AtomicOrdering};
use deque;
use service_mio::{HandlerId, IoChannel, IoContext};
use tokio::{self};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it is safer to keep this self to be backward compatible but these are not needed since Rust 1.30

util/io/src/worker.rs Outdated Show resolved Hide resolved
util/io/Cargo.toml Outdated Show resolved Hide resolved
util/io/src/worker.rs Outdated Show resolved Hide resolved
@ordian ordian added this to the 2.3 milestone Nov 28, 2018
@grbIzl grbIzl added A0-pleasereview 🤓 Pull request needs code review. and removed A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Nov 30, 2018
@sorpaas sorpaas added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 3, 2018
@sorpaas sorpaas merged commit 869fa39 into master Dec 3, 2018
@sorpaas sorpaas deleted the TobalabaPrivateFixes branch December 3, 2018 14:35
niklasad1 pushed a commit that referenced this pull request Dec 16, 2018
* Add tokio runtime to ethcore io worker

* Reworked with block_on_all
insipx pushed a commit to insipx/parity-ethereum that referenced this pull request Dec 17, 2018
* Add tokio runtime to ethcore io worker

* Reworked with block_on_all
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants