Skip to content

Commit

Permalink
Revert "src: let http2 streams end after session close"
Browse files Browse the repository at this point in the history
This reverts commit dee882e.
Moved the test that demonstrated what this commit was fixing to the
`known_issues` folder.

Fixes: nodejs#46234
  • Loading branch information
santigimeno authored and Trott committed Feb 19, 2023
1 parent b4a962d commit 9285979
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 13 deletions.
11 changes: 0 additions & 11 deletions src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1124,17 +1124,6 @@ int Http2Session::OnStreamClose(nghttp2_session* handle,
if (!stream || stream->is_destroyed())
return 0;

// Don't close synchronously in case there's pending data to be written. This
// may happen when writing trailing headers.
if (code == NGHTTP2_NO_ERROR && nghttp2_session_want_write(handle) &&
env->can_call_into_js()) {
env->SetImmediate([handle, id, code, user_data](Environment* env) {
OnStreamClose(handle, id, code, user_data);
});

return 0;
}

stream->Close(code);

// It is possible for the stream close to occur before the stream is
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
'use strict';

// Fixes: https://github.com/nodejs/node/issues/42713
const common = require('../common');
if (!common.hasCrypto)
if (!common.hasCrypto) {
// Remove require('assert').fail when issue is fixed and test
// is moved out of the known_issues directory.
require('assert').fail('missing crypto');
common.skip('missing crypto');
}
const assert = require('assert');
const http2 = require('http2');

Expand Down Expand Up @@ -31,7 +36,7 @@ server.listen(0, common.mustCall(() => {
client.socket.on('close', common.mustCall());
const req = client.request({
[HTTP2_HEADER_PATH]: '/',
[HTTP2_HEADER_METHOD]: 'POST'
[HTTP2_HEADER_METHOD]: 'POST',
});
req.end();
req.on('response', common.mustCall());
Expand Down

0 comments on commit 9285979

Please sign in to comment.