8000 fix(node): fix sharing provider between async runtimes by GCdePaula · Pull Request #168 · cartesi/dave · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
8000

fix(node): fix sharing provider between async runtimes #168

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

Merged
merged 1 commit into from
May 20, 2025

Conversation

GCdePaula
Copy link
Collaborator

No description provided.

@GCdePaula GCdePaula requested a review from stephenctw May 19, 2025 20:36
@GCdePaula GCdePaula force-pushed the fix/provider-sharing branch from 1fd2d85 to 729dcf3 Compare May 19, 2025 20:40
@GCdePaula GCdePaula force-pushed the fix/provider-sharing branch from 729dcf3 to dc4fc7c Compare May 19, 2025 20:42
@stephenctw
Copy link
Collaborator
stephenctw commented May 20, 2025

@GCdePaula Could you briefly explain what was broken and trying to fix in this PR?

@GCdePaula
Copy link
Collaborator Author
GCdePaula commented May 20, 2025

Yeah! There's been an intermittent transport error TransportError(…, hyper::Error(IncompleteMessage)), which is making the node crash.

After investigating, the issue seems to be related to the latest tokio rework.

Currently, we instantiate one single provider in tokio runtime A. This runtime then ends, and we instantiate runtimes B and C.

Both B and C receive the provider created by now-dead A.

It seems request+hyper expect that everything be in the same runtime. Or at least that the runtime that created everything doesn't die. Possibly both. The point is that sharing causes issues. Here's a related issue with a comment by the owner of request.

This patch instantiates one provider per service/worker, inside the worker's own tokio runtime.

Copy link
Collaborator
@stephenctw stephenctw left a comment

Choose a reason for hiding this comment

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

Looks great and all tests passed on my side locally. I wonder how to verify the issue is being addressed. Just put it running on testnet?

@GCdePaula
Copy link
Collaborator Author

Yeah, my plan was creating a new release and seeing if the test deployment stops crashing.

@GCdePaula GCdePaula merged commit 6eb7498 into main May 20, 2025
6 checks passed
@GCdePaula GCdePaula deleted the fix/provider-sharing branch May 20, 2025 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0