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

n-api: guard against cond null dereference #21871

Conversation

gabrielschulhof
Copy link
Contributor

A condition variable is only created by the thread-safe function if the
queue size is set to something larger than zero. This adds tests for the
case where the queue size is zero.

Fixes: nodejs/help#1387

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@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 node-api Issues and PRs related to the Node-API. labels Jul 18, 2018
@gabrielschulhof
Copy link
Contributor Author

CI to flush out potential platform-dependent bugs: https://ci.nodejs.org/job/node-test-pull-request/15937/

@gabrielschulhof gabrielschulhof force-pushed the help-1387-tsfn-zero-queue branch from bdc2b60 to 837e716 Compare July 18, 2018 16:27
@gabrielschulhof
Copy link
Contributor Author

Another CI after fixing syntax: https://ci.nodejs.org/job/node-test-pull-request/15938/

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

A condition variable is only created by the thread-safe function if the
queue size is set to something larger than zero. This adds null-checks
around the condition variable and tests for the case where the queue
size is zero.

Fixes: nodejs/help#1387
@gabrielschulhof gabrielschulhof force-pushed the help-1387-tsfn-zero-queue branch from 837e716 to 6f71428 Compare July 19, 2018 02:25
@gabrielschulhof
Copy link
Contributor Author

Improved the commit message a bit.

@gabrielschulhof
Copy link
Contributor Author

@gabrielschulhof
Copy link
Contributor Author

@nodejs/collaborators may I please have another review before landing this?

@gabrielschulhof
Copy link
Contributor Author

@gabrielschulhof
Copy link
Contributor Author

Landed in 53296e8.

gabrielschulhof pushed a commit that referenced this pull request Jul 24, 2018
A condition variable is only created by the thread-safe function if the
queue size is set to something larger than zero. This adds null-checks
around the condition variable and tests for the case where the queue
size is zero.

Fixes: nodejs/help#1387
PR-URL: #21871
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@gabrielschulhof gabrielschulhof deleted the help-1387-tsfn-zero-queue branch July 24, 2018 02:09
targos pushed a commit that referenced this pull request Jul 24, 2018
A condition variable is only created by the thread-safe function if the
queue size is set to something larger than zero. This adds null-checks
around the condition variable and tests for the case where the queue
size is zero.

Fixes: nodejs/help#1387
PR-URL: #21871
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@targos targos mentioned this pull request Jul 31, 2018
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Dec 28, 2018
A condition variable is only created by the thread-safe function if the
queue size is set to something larger than zero. This adds null-checks
around the condition variable and tests for the case where the queue
size is zero.

Fixes: nodejs/help#1387
PR-URL: nodejs#21871
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Jan 18, 2019
A condition variable is only created by the thread-safe function if the
queue size is set to something larger than zero. This adds null-checks
around the condition variable and tests for the case where the queue
size is zero.

Fixes: nodejs/help#1387
PR-URL: #21871
Backport-PR-URL: #25002
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
rvagg pushed a commit that referenced this pull request Feb 28, 2019
A condition variable is only created by the thread-safe function if the
queue size is set to something larger than zero. This adds null-checks
around the condition variable and tests for the case where the queue
size is zero.

Fixes: nodejs/help#1387
PR-URL: #21871
Backport-PR-URL: #25002
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@MylesBorins MylesBorins mentioned this pull request Mar 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. node-api Issues and PRs related to the Node-API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

napi_threadsafe_function -- Segmentation fault: 11
6 participants