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.

Sign up for GitHub

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

Jump to bottom

Pipelining and Socket handlers #38

Closed
calebboyd opened this issue Apr 4, 2021 · 4 comments
Closed

Pipelining and Socket handlers #38

calebboyd opened this issue Apr 4, 2021 · 4 comments

Comments

@calebboyd
Copy link

calebboyd commented Apr 4, 2021 •
edited
Loading

I noticed that the same issue fixed here occurs when a client is pipelining requests to a node server using on-finished for response messages. The pipelined requests cause the response to have the same socket instance, which on-finished is repeatedly attaching new handlers to.

Would a PR adding tests for this case and implementing the same approach (queueing listeners) be considered?
@dougwilson
Copy link
Contributor

Yes, definitely. Even just a test case is great to get if you're not sure on the implementation.

@calebboyd calebboyd mentioned this issue Apr 4, 2021
Closed
@calebboyd
Copy link
Author

Cool, I added a test case -- I was looking at the CI matrix and it looks like if I add an implementation it will have to work in Node 0.8?

@dougwilson
Copy link
Contributor

Thank you! Yes, that is correct, but I am happy to help make that work if you cannot get it there.

Also to note, it seems that the test case you added in that PR is passing, so it would seem that either it doesn't replicate the issue, there is something wrong in the test case, or some other variable in play not replicated in the test case.

@calebboyd
Copy link
Author

Thats weird - they were failing on my mac with a lower number of pipelined requests. Upping the number has them now failing in CI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

reduce listeners added to pipelining socket calebboyd/on-finished
2 participants
@dougwilson @calebboyd