Skip to content
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

Deadlock when request is issued when some subscription is active #143

Closed
svyatonik opened this issue Oct 27, 2020 · 3 comments
Closed

Deadlock when request is issued when some subscription is active #143

svyatonik opened this issue Oct 27, 2020 · 3 comments
Labels
bug Something isn't working

Comments

@svyatonik
Copy link
Contributor

Steps to repeat:

  1. open a connection to the ws-server (e.g. substrate node);
  2. subscribe to some notifications stream (e.g. to justifications using grandpa_justifications);
  3. wait enough time for 5 notifications to be sent by the server (e.g. 20 blocks). DO NOT read any notifications from jsonrpsee::Subscription;
  4. make request to the server (e.g. chain_getHeader) using the same client => deadlock.

The reason is that notifications channel is created with buffer size = 4. When we're trying to send 5th notification using blocking SinkExt::send().await here, the background thread will stop processing FrontToBack requests. So if we have single-threaded client, it'll block here, waiting for background thread to issue request and read response. At the same time, background thread is waiting for us to read pending notifications from the channel.

@niklasad1
Copy link
Member

Right, so the simple fix is to use an unbounded channel?

It seems tricky to pick the "right" buffer size.

@tomusdrw
Copy link
Contributor

tomusdrw commented Oct 27, 2020

IMHO have it user-configured and drop excessive notifications (they usually have an ephemeral nature anyway). If the user is interested in not missing any of them, then they can set the size of the channel high enough. We can't (and imho shouldn't) really back-pressure the server, it would presumably be dropping excessive notifications if the client can't keep up either (or even dropping the connection) - you can't stop the external stuff that generates these notifications from making progress really :)

@niklasad1
Copy link
Member

Fixed by #180

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants