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

net: partially revert "simplify Socket.prototype._final" #24288

Closed
wants to merge 1 commit into from

Conversation

addaleax
Copy link
Member

Partially revert b7e6ccd because it broke a test that was added since its last CI run.

Refs: #24075
Refs: #23866

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

Partially revert b7e6ccd
because it broke a test that was added since its last CI run.

Refs: nodejs#24075
Refs: nodejs#23866
@nodejs-github-bot nodejs-github-bot added the net Issues and PRs related to the net subsystem. label Nov 10, 2018
@addaleax
Copy link
Member Author

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

Please 👍 this comment to approve fast-tracking.

(I have what I think is a proper solution for the test, but it’s a bit more complex than something I’d be willing to have fast-tracked myself.)

@addaleax addaleax added the fast-track PRs that do not need to wait for 48 hours to land. label Nov 10, 2018
@addaleax
Copy link
Member Author

(hmm … @nodejs/collaborators ?)

@addaleax
Copy link
Member Author

Landed in fb6c669, thanks @cjihrig and @ryzokuken!

@addaleax addaleax closed this Nov 10, 2018
@addaleax addaleax deleted the unbreak-master branch November 10, 2018 20:54
addaleax added a commit that referenced this pull request Nov 10, 2018
Partially revert b7e6ccd
because it broke a test that was added since its last CI run.

Refs: #24075
Refs: #23866

PR-URL: #24288
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
addaleax added a commit to addaleax/node that referenced this pull request Nov 10, 2018
BridgeAR pushed a commit that referenced this pull request Nov 14, 2018
Partially revert b7e6ccd
because it broke a test that was added since its last CI run.

Refs: #24075
Refs: #23866

PR-URL: #24288
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
kiyomizumia pushed a commit to kiyomizumia/node that referenced this pull request Nov 15, 2018
Partially revert b7e6ccd
because it broke a test that was added since its last CI run.

Refs: nodejs#24075
Refs: nodejs#23866

PR-URL: nodejs#24288
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
addaleax added a commit that referenced this pull request Nov 16, 2018
`'drain'` event handlers may not be invoked if the stream
is currently finishing. Instead, use the fact that we know
when writes are active or not, and invoke the delayed shutdown
handler from our own after-write callback.

PR-URL: #24290
Refs: #24288
Refs: #24075
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Ouyang Yadong <oyydoibh@gmail.com>
addaleax added a commit that referenced this pull request Nov 16, 2018
Previously, there was no mechanism in place that would
have destroyed the TLS socket once the underlying socket
had been closed.

PR-URL: #24290
Refs: #24288
Refs: #24075
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Ouyang Yadong <oyydoibh@gmail.com>
addaleax added a commit that referenced this pull request Nov 16, 2018
This reverts commit ac1f56c.

Refs: #24288
Refs: #24075

PR-URL: #24290
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Ouyang Yadong <oyydoibh@gmail.com>
targos pushed a commit that referenced this pull request Nov 18, 2018
`'drain'` event handlers may not be invoked if the stream
is currently finishing. Instead, use the fact that we know
when writes are active or not, and invoke the delayed shutdown
handler from our own after-write callback.

PR-URL: #24290
Refs: #24288
Refs: #24075
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Ouyang Yadong <oyydoibh@gmail.com>
targos pushed a commit that referenced this pull request Nov 18, 2018
Previously, there was no mechanism in place that would
have destroyed the TLS socket once the underlying socket
had been closed.

PR-URL: #24290
Refs: #24288
Refs: #24075
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Ouyang Yadong <oyydoibh@gmail.com>
targos pushed a commit that referenced this pull request Nov 18, 2018
This reverts commit ac1f56c.

Refs: #24288
Refs: #24075

PR-URL: #24290
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Ouyang Yadong <oyydoibh@gmail.com>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
`'drain'` event handlers may not be invoked if the stream
is currently finishing. Instead, use the fact that we know
when writes are active or not, and invoke the delayed shutdown
handler from our own after-write callback.

PR-URL: #24290
Refs: #24288
Refs: #24075
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Ouyang Yadong <oyydoibh@gmail.com>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
Previously, there was no mechanism in place that would
have destroyed the TLS socket once the underlying socket
had been closed.

PR-URL: #24290
Refs: #24288
Refs: #24075
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Ouyang Yadong <oyydoibh@gmail.com>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
This reverts commit ac1f56c.

Refs: #24288
Refs: #24075

PR-URL: #24290
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Ouyang Yadong <oyydoibh@gmail.com>
codebytere pushed a commit that referenced this pull request Dec 14, 2018
Partially revert b7e6ccd
because it broke a test that was added since its last CI run.

Refs: #24075
Refs: #23866

PR-URL: #24288
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 26, 2018
Partially revert b7e6ccd
because it broke a test that was added since its last CI run.

Refs: #24075
Refs: #23866

PR-URL: #24288
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
@codebytere codebytere mentioned this pull request Jan 4, 2019
codebytere pushed a commit that referenced this pull request Jan 12, 2019
`'drain'` event handlers may not be invoked if the stream
is currently finishing. Instead, use the fact that we know
when writes are active or not, and invoke the delayed shutdown
handler from our own after-write callback.

PR-URL: #24290
Refs: #24288
Refs: #24075
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Ouyang Yadong <oyydoibh@gmail.com>
codebytere pushed a commit that referenced this pull request Jan 12, 2019
Previously, there was no mechanism in place that would
have destroyed the TLS socket once the underlying socket
had been closed.

PR-URL: #24290
Refs: #24288
Refs: #24075
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Ouyang Yadong <oyydoibh@gmail.com>
codebytere pushed a commit that referenced this pull request Jan 12, 2019
This reverts commit ac1f56c.

Refs: #24288
Refs: #24075

PR-URL: #24290
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Ouyang Yadong <oyydoibh@gmail.com>
codebytere pushed a commit that referenced this pull request Jan 29, 2019
`'drain'` event handlers may not be invoked if the stream
is currently finishing. Instead, use the fact that we know
when writes are active or not, and invoke the delayed shutdown
handler from our own after-write callback.

PR-URL: #24290
Refs: #24288
Refs: #24075
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Ouyang Yadong <oyydoibh@gmail.com>
codebytere pushed a commit that referenced this pull request Jan 29, 2019
Previously, there was no mechanism in place that would
have destroyed the TLS socket once the underlying socket
had been closed.

PR-URL: #24290
Refs: #24288
Refs: #24075
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Ouyang Yadong <oyydoibh@gmail.com>
codebytere pushed a commit that referenced this pull request Jan 29, 2019
This reverts commit ac1f56c.

Refs: #24288
Refs: #24075

PR-URL: #24290
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Ouyang Yadong <oyydoibh@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fast-track PRs that do not need to wait for 48 hours to land. net Issues and PRs related to the net subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants