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: #46234
PR-URL: #46721
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
  • Loading branch information
santigimeno authored and juanarbol committed Mar 5, 2023
1 parent 1ef3c53 commit 73a8f46
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
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
@@ -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 73a8f46

Please sign in to comment.