-
Notifications
You must be signed in to change notification settings - Fork 462
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. |
Fixes: #1150 PR-URL: #1168 Reviewed-By: Michael Dawson <[email protected]
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 <[email protected]
Fixes: #1150 PR-URL: nodejs/node-addon-api#1168 Reviewed-By: Michael Dawson <[email protected]
Fixes: #1150 PR-URL: nodejs/node-addon-api#1168 Reviewed-By: Michael Dawson <[email protected]
Fixes: #1150 PR-URL: nodejs/node-addon-api#1168 Reviewed-By: Michael Dawson <[email protected]
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