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

Fix scope issue when setting thread names in the multi-threaded signature checker #1674

Merged

Conversation

Projects
2 participants
@wezrule
Copy link
Collaborator

commented Jan 31, 2019

There are some intermittent test failures (I could only reproduce them on a MAC), caused by a SIGABRT in the ~std::unique_ptr destructor. It seems that the mutex is going out of scope before the thread pool has fully finished. When the last promise.set_value () is called it notifies the main thread that the thread work is finished, and then exits the function (set_thread_names), but the threads haven't fully finished (i.e they still need to call destructors). I'm now (re)using the class member mutex so that it stays in the scope during this cleanup.

@wezrule wezrule added the bug label Jan 31, 2019

@wezrule wezrule added this to the V18.0 milestone Jan 31, 2019

@wezrule wezrule self-assigned this Jan 31, 2019

@wezrule wezrule requested a review from cryptocode Jan 31, 2019

@cryptocode
Copy link
Collaborator

left a comment

LGTM. I get #cores - 1 signature checker threads now, I believe this was #cores the first time I tested this, but looks like this change was made a few commits ago.

@wezrule

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 31, 2019

Thanks! Yeh this was changed because the calling thread now does work, so num_threads is 'extra' threads doing work.

@wezrule wezrule merged commit 65b2584 into nanocurrency:master Jan 31, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@wezrule wezrule deleted the wezrule:fix_data_race_multithreaded_sig_checker branch Jan 31, 2019

@zhyatt zhyatt added this to During RC in V18 Feb 6, 2019

@zhyatt zhyatt moved this from RC2 to CP 3/RC 1 (2018-02-01) in V18 Feb 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.