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

http2: when is socket._paused set to true? #16213

Closed
trivikr opened this issue Oct 15, 2017 · 7 comments
Closed

http2: when is socket._paused set to true? #16213

trivikr opened this issue Oct 15, 2017 · 7 comments
Labels
http2 Issues or PRs related to the http2 subsystem. question Issues that look for answers.

Comments

@trivikr
Copy link
Member

trivikr commented Oct 15, 2017

As part of improving code coverage for http2 in #14985, I wrote some unit tests for socket operations in #16211

I noticed that:

However, I didn't find any code which sets socket._paused to true https://github.com/nodejs/node/search?utf8=%E2%9C%93&q=_paused&type=
Is this a bug? Or is there a way to mock value of socket._paused while testing?

@mscdex mscdex added http2 Issues or PRs related to the http2 subsystem. question Issues that look for answers. labels Oct 15, 2017
@apapirovski
Copy link
Member

cc @jasnell — this seems like an incomplete implementation or a bug, any insight into what the original thinking was?

@trivikr
Copy link
Member Author

trivikr commented Oct 15, 2017

For reference, initial commit which introduced _paused: nodejs/http2@4e47406

@trivikr
Copy link
Member Author

trivikr commented Oct 17, 2017

This issue is likely to be irrelevant if socket operations are removed in #16252

@Fishrock123
Copy link
Contributor

Isn't this part of the streams implementation?

@apapirovski
Copy link
Member

@Fishrock123 as far as I can tell it's custom to _http_server.js

@trivikr
Copy link
Member Author

trivikr commented Oct 23, 2017

The socketOn* methods are being removed in #16330
This issue will cease to exist when the PR is merged.

@apapirovski
Copy link
Member

apapirovski commented Oct 25, 2017

We still have unused stream._paused but this issue isn't addressing that so I'm closing since the PR removing this just landed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http2 Issues or PRs related to the http2 subsystem. question Issues that look for answers.
Projects
None yet
Development

No branches or pull requests

4 participants