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

v4.x backport: http: make request.abort() destroy the socket #11765

Closed
wants to merge 1 commit into from
Closed

v4.x backport: http: make request.abort() destroy the socket #11765

wants to merge 1 commit into from

Conversation

lpinca
Copy link
Member

@lpinca lpinca commented Mar 9, 2017 •
edited
Loading

v4.x backport of #10818.

Commit landed cleanly and tests pass on my machine (macOS 10.11.6).
I'm opening this PR as per this comment.

CI: https://ci.nodejs.org/job/node-test-pull-request/6758/

@lpinca
http: make request.abort() destroy the socket
b36e136 
`request.abort()` did not destroy the socket if it was called
before a socket was assigned to the request and the request
did not use an `Agent` or a Unix Domain Socket was used.

Fixes: nodejs#10812
PR-URL: nodejs#10818
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
@nodejs-github-bot nodejs-github-bot added http Issues or PRs related to the http subsystem. v4.x labels Mar 9, 2017
@lpinca
Copy link
Member Author

lpinca commented Mar 9, 2017

cc: @MylesBorins

It would be nice to have this in v4.8.1

@MylesBorins
Copy link
Contributor

looks like this is still having failures. It is possible that the next v4.x release will be the last Active LTS release of v4 and an RC has already gone out. You will need to get the tests passing ASAP if we are going to try and roll it into v4.8.1

@lpinca
Copy link
Member Author

lpinca commented Mar 9, 2017 •
edited
Loading

@MylesBorins the only failures I can see are

not ok 1122 sequential/test-fs-watch
  ---
  duration_ms: 60.88
  severity: fail
  stack: |-
    timeout

on osx and

not ok 875 parallel/test-tick-processor # TODO : Fix flaky test
  ---
  duration_ms: 10.426
  severity: flaky
  stack: |-

on ppcle-ubuntu1404.
Both seem unrelated to me.

@lpinca
Copy link
Member Author

lpinca commented Mar 9, 2017

CI 2: https://ci.nodejs.org/job/node-test-pull-request/6775/

@lpinca
Copy link
Member Author

lpinca commented Mar 10, 2017 •
edited
Loading

CI is 100% green.

@MylesBorins
Copy link
Contributor

landed in 0fe9ac3

@lpinca lpinca deleted the backport/10818 branch March 11, 2017 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jasnell jasnell

@MylesBorins MylesBorins

Labels
http Issues or PRs related to the http subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lpinca @MylesBorins @jasnell @nodejs-github-bot