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
HPCC-23183 Thread local hook fix when on main thread #13197
HPCC-23183 Thread local hook fix when on main thread #13197
Conversation
https://track.hpccsystems.com/browse/HPCC-23183 |
@richardkchapman - please review. |
b6c75d3
to
06737ec
Compare
@richardkchapman - modified so that callThreadTerminationHooks handles the condition based on main thread id. |
{ | ||
tid = GetCurrentThreadId(); | ||
} | ||
} mainThreadIdHelper; |
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.
should be static?
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.
agree, have changed.
One minor comment. Also would be worth adding a comment about why this is necessary (i.e. that thread-local objects are destroyed before functions called via atexit ) @ghalliday can you sanity check too |
Track hooks installed on main thread separately, in a non thread locl vector. Signed-off-by: Jake Smith <jake.smith@lexisnexisrisk.com>
06737ec
to
7897e37
Compare
made mainThreadIdHelper static and added a comment. @ghalliday - please review. |
Automated Smoketest: ✅ Unit tests result:
Regression test result:
HPCC Stop: OK
|
@@ -41,26 +41,45 @@ | |||
|
|||
//static __thread ThreadTermFunc threadTerminationHook; |
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.
This line should have been removed by my previous PR in tis area..
Track hooks installed on main thread separately, in a non
thread locl vector.
Signed-off-by: Jake Smith jake.smith@lexisnexisrisk.com
Type of change:
Checklist:
Smoketest:
Testing: