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

src: add nullptr check for session in DEBUG macro #18815

Closed
wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Feb 16, 2018

Currenlty when configuring --debug-http2
test/parallel/test-http2-getpackedsettings.js will segment fault:

$ out/Debug/node test/parallel/test-http2-getpackedsettings.js
Segmentation fault: 11

This is happening because the settings is created with the Environment in
PackSettings:

Http2Session::Http2Settings settings(env);

This will cause the session to be set to nullptr. When the init
function is later called the expanded DEBUG_HTTP2SESSION macro will
cause the segment fault when the session is dereferenced.

I'm not sure about what the proper solution is here as I'm not very
familiar with the http2 code yet. Creating this so that others can
provide some feedback.

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

src, http2

Currenlty when configuring --debug-http2
/test/parallel/test-http2-getpackedsettings.js will segment fault:

$ out/Debug/node test/parallel/test-http2-getpackedsettings.js
Segmentation fault: 11

This is happening because the settings is created with the Environment in
PackSettings:
Http2Session::Http2Settings settings(env);
This will cause the session to be set to nullptr. When the init
function is later called the expanded DEBUG_HTTP2SESSION macro will
cause the segment fault when the session is dereferenced.

I'm not sure about what the proper solution is here as I'm not very
familiar with the http2 code yet. Creating this so that others can
provide some feedback.
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. dont-land-on-v4.x http2 Issues or PRs related to the http2 subsystem. labels Feb 16, 2018
@danbev
Copy link
Contributor Author

danbev commented Feb 16, 2018

@richardlau
Copy link
Member

cc @nodejs/http2

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 16, 2018
@danbev
Copy link
Contributor Author

danbev commented Feb 19, 2018

node-test-commit failure looks unrelated

console output:

`which node`  tools/doc/generate.js --format=json doc/api/zlib.md > out/doc/api/zlib.json; else echo "No available node, cannot run \"node  tools/doc/generate.js --format=json doc/api/zlib.md > out/doc/api/zlib.json\""; exit 1; fi;
make[2]: Leaving directory `/home/iojs/build/workspace/node-test-commit-plinux/nodes/ppcle-ubuntu1404'
make[2]: write error
make[1]: *** [doc-only] Error 1
make[1]: Leaving directory `/home/iojs/build/workspace/node-test-commit-plinux/nodes/ppcle-ubuntu1404'
make: *** [run-ci] Error 2
Process leaked file descriptors. See https://jenkins.io/redirect/troubleshooting/process-leaked-file-descriptors for more information
Build step 'Execute shell' marked build as failure
Run condition [Always] enabling perform for step [[]]
TAP Reports Processing: START
Looking for TAP results report in workspace using pattern: *.tap
Did not find any matching files. Setting build result to FAILURE.
Checking ^not ok
Jenkins Text Finder: File set '*.tap' is empty
Sending e-mails to: michael_dawson@ca.ibm.com gib@uk.ibm.com
Notifying upstream projects of job completion
Finished: FAILURE

@danbev
Copy link
Contributor Author

danbev commented Feb 19, 2018

Landed in e91ea21

@danbev danbev closed this Feb 19, 2018
danbev added a commit to danbev/node that referenced this pull request Feb 19, 2018
Currenlty when configuring --debug-http2
/test/parallel/test-http2-getpackedsettings.js will segment fault:

$ out/Debug/node test/parallel/test-http2-getpackedsettings.js
Segmentation fault: 11

This is happening because the settings is created with the Environment in
PackSettings:
Http2Session::Http2Settings settings(env);
This will cause the session to be set to nullptr. When the init
function is later called the expanded DEBUG_HTTP2SESSION macro will
cause the segment fault when the session is dereferenced.

PR-URL: nodejs#18815
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@danbev danbev deleted the http2_packed_setting_debug branch February 19, 2018 07:23
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
Currenlty when configuring --debug-http2
/test/parallel/test-http2-getpackedsettings.js will segment fault:

$ out/Debug/node test/parallel/test-http2-getpackedsettings.js
Segmentation fault: 11

This is happening because the settings is created with the Environment in
PackSettings:
Http2Session::Http2Settings settings(env);
This will cause the session to be set to nullptr. When the init
function is later called the expanded DEBUG_HTTP2SESSION macro will
cause the segment fault when the session is dereferenced.

PR-URL: #18815
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Feb 21, 2018
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
Currenlty when configuring --debug-http2
/test/parallel/test-http2-getpackedsettings.js will segment fault:

$ out/Debug/node test/parallel/test-http2-getpackedsettings.js
Segmentation fault: 11

This is happening because the settings is created with the Environment in
PackSettings:
Http2Session::Http2Settings settings(env);
This will cause the session to be set to nullptr. When the init
function is later called the expanded DEBUG_HTTP2SESSION macro will
cause the segment fault when the session is dereferenced.

PR-URL: #18815
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
Currenlty when configuring --debug-http2
/test/parallel/test-http2-getpackedsettings.js will segment fault:

$ out/Debug/node test/parallel/test-http2-getpackedsettings.js
Segmentation fault: 11

This is happening because the settings is created with the Environment in
PackSettings:
Http2Session::Http2Settings settings(env);
This will cause the session to be set to nullptr. When the init
function is later called the expanded DEBUG_HTTP2SESSION macro will
cause the segment fault when the session is dereferenced.

PR-URL: #18815
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
kjin pushed a commit to kjin/node that referenced this pull request May 1, 2018
Currenlty when configuring --debug-http2
/test/parallel/test-http2-getpackedsettings.js will segment fault:

$ out/Debug/node test/parallel/test-http2-getpackedsettings.js
Segmentation fault: 11

This is happening because the settings is created with the Environment in
PackSettings:
Http2Session::Http2Settings settings(env);
This will cause the session to be set to nullptr. When the init
function is later called the expanded DEBUG_HTTP2SESSION macro will
cause the segment fault when the session is dereferenced.

PR-URL: nodejs#18815
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 2, 2018
Currenlty when configuring --debug-http2
/test/parallel/test-http2-getpackedsettings.js will segment fault:

$ out/Debug/node test/parallel/test-http2-getpackedsettings.js
Segmentation fault: 11

This is happening because the settings is created with the Environment in
PackSettings:
Http2Session::Http2Settings settings(env);
This will cause the session to be set to nullptr. When the init
function is later called the expanded DEBUG_HTTP2SESSION macro will
cause the segment fault when the session is dereferenced.

Backport-PR-URL: #20456
PR-URL: #18815
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request May 2, 2018
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
Currenlty when configuring --debug-http2
/test/parallel/test-http2-getpackedsettings.js will segment fault:

$ out/Debug/node test/parallel/test-http2-getpackedsettings.js
Segmentation fault: 11

This is happening because the settings is created with the Environment in
PackSettings:
Http2Session::Http2Settings settings(env);
This will cause the session to be set to nullptr. When the init
function is later called the expanded DEBUG_HTTP2SESSION macro will
cause the segment fault when the session is dereferenced.

PR-URL: nodejs#18815
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 15, 2018
Currenlty when configuring --debug-http2
/test/parallel/test-http2-getpackedsettings.js will segment fault:

$ out/Debug/node test/parallel/test-http2-getpackedsettings.js
Segmentation fault: 11

This is happening because the settings is created with the Environment in
PackSettings:
Http2Session::Http2Settings settings(env);
This will cause the session to be set to nullptr. When the init
function is later called the expanded DEBUG_HTTP2SESSION macro will
cause the segment fault when the session is dereferenced.

Backport-PR-URL: #20456
PR-URL: #18815
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 15, 2018
Currenlty when configuring --debug-http2
/test/parallel/test-http2-getpackedsettings.js will segment fault:

$ out/Debug/node test/parallel/test-http2-getpackedsettings.js
Segmentation fault: 11

This is happening because the settings is created with the Environment in
PackSettings:
Http2Session::Http2Settings settings(env);
This will cause the session to be set to nullptr. When the init
function is later called the expanded DEBUG_HTTP2SESSION macro will
cause the segment fault when the session is dereferenced.

Backport-PR-URL: #20456
PR-URL: #18815
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. http2 Issues or PRs related to the http2 subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants