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

reduce listeners added to pipelining socket #39

Closed
wants to merge 3 commits into from
Closed

reduce listeners added to pipelining socket #39

wants to merge 3 commits into from

Conversation

calebboyd
Copy link

@calebboyd calebboyd commented Apr 4, 2021 •
edited
Loading

fixes #38

@calebboyd calebboyd force-pushed the master branch 2 times, most recently from dae078f to 558d27d Compare April 4, 2021 22:40
@calebboyd calebboyd marked this pull request as ready for review April 5, 2021 02:19
@calebboyd calebboyd changed the title test: add failing test for pipelining client reduce listeners added to pipelining socket Apr 5, 2021
calebboyd
calebboyd commented Apr 5, 2021
View reviewed changes
index.js Outdated Show resolved Hide resolved
@calebboyd calebboyd force-pushed the master branch 6 times, most recently from dc18131 to be92047 Compare April 6, 2021 00:00
@calebboyd
Copy link
Author

calebboyd commented Apr 9, 2021 •
edited
Loading

@dougwilson This implementation should be correct - The socket queue listener is held until the socket is closed/ended. No guard is needed in the onFinish callback because the listener has an empty queue after it is executed.

@calebboyd calebboyd closed this May 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dougwilson dougwilson dougwilson left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Pipelining and Socket handlers
2 participants
@calebboyd @dougwilson