Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

async usage of svm-rs #1259

Closed
rkrasiuk opened this issue May 13, 2022 · 6 comments
Closed

async usage of svm-rs #1259

rkrasiuk opened this issue May 13, 2022 · 6 comments

Comments

@rkrasiuk
Copy link
Contributor

Is your feature request related to a problem? Please describe.
usage of svm-rs with a blocking feature inside a tokio runtime leads to the following error
Cannot drop a runtime in a context where blocking is not allowed. This happens when a runtime is dropped from within an asynchronous context.
reqwest::blocking doesn't behave well inside another tokio rt

Describe the solution you'd like
use svm async threadbound

cc @mattsse

@mattsse
Copy link
Collaborator

mattsse commented May 13, 2022

this is unfortunatelly a limitation of reqwest::blocking

what we could do

  1. use svm-async within
 runtime::Builder::new_current_thread()
                .enable_io()
                .build().unwrap().block_on(async {svm::install})
  1. try accessing the Handle or create a new multithreaded rt

actually not sure about the performance benefits of 2 over 1

@gakonst
Copy link
Owner

gakonst commented May 13, 2022

OK makes sense, so reqwest::blocking doesn't behave well in Tokio environments, would have been nice if it did.

I think let's go with:

  1. https://github.com/foundry-rs/foundry/blob/master/utils/src/lib.rs#L27-L54 move this over to ethers-solc (and expose it so we can import it in foundry)
  2. Use RuntimeOrHandle::new().block_on(...) for the 2 fns where blocking_install is called:

@mattsse mentioned there might be some performance change if using single vs multi threaded runtime, but I think this is a case where it shouldn't matter that much, given that it's a single request.

@rkrasiuk
Copy link
Contributor Author

@gakonst and why don't we block_on the actual svm::blocking_install?

match svm::blocking_install(version) {

@gakonst
Copy link
Owner

gakonst commented May 13, 2022

I guess that also works :D

@mattsse
Copy link
Collaborator

mattsse commented May 16, 2022

vote close @gakonst

@rkrasiuk
Copy link
Contributor Author

second @mattsse. my foundry integration tests seem to be fixed

@gakonst gakonst closed this as completed May 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants