-
Notifications
You must be signed in to change notification settings - Fork 459
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
test: Check for tsfn in condition_variable wait #1168
Conversation
Run on 12.x where it seems to fail the most - https://ci.nodejs.org/view/x%20-%20Abi%20stable%20module%20API/job/node-test-node-addon-api-new/5783/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The CI runs that I started looked good. I'll go ahead and land as it should help get the ci green :). @KevinEady thanks for working on this. |
Landed in 24455f8 |
Whoo-hoo a full green CI run across Node.js runs after landing this - https://ci.nodejs.org/view/x%20-%20Abi%20stable%20module%20API/job/node-test-node-addon-api-LTS%20versions/1481/ |
Fixes: #1150 PR-URL: nodejs/node-addon-api#1168 Reviewed-By: Michael Dawson <midawson@redhat.com
Fixes: #1150 PR-URL: nodejs/node-addon-api#1168 Reviewed-By: Michael Dawson <midawson@redhat.com
Fixes: #1150 PR-URL: nodejs/node-addon-api#1168 Reviewed-By: Michael Dawson <midawson@redhat.com
Fixes: #1150 PR-URL: nodejs/node-addon-api#1168 Reviewed-By: Michael Dawson <midawson@redhat.com
Under certain conditions, the main thread may set the TSFN and notify the condition variable prior to the thread waiting on it, causing an indefinite hang. This PR fixes the hang by skipping the wait if the TSFN has been set.
Fixes: #1150