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
rec: Make threads run until asked to stop. #8521
rec: Make threads run until asked to stop. #8521
Conversation
This is safer since the atexit handler is not ran while threads are still active. Also, when using valgrind we get more clean leak reports. Retry if the accidentally merged PowerDNS#8518 that was reverted.
With this (on a short run): ==13452== HEAP SUMMARY: ==13452== in use at exit: 0 bytes in 0 blocks ==13452== total heap usage: 54,657 allocs, 54,657 frees, 14,008,997 bytes allocated
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.
In theory I quite like this change, but I'm a bit worried we might experience the recursor not exiting for a long time in practice, which might be painful. Let's merge it once we are sure that the initial value of RecursorControlChannel::stop
is set and see how it goes.
pdns/rec_channel.cc
Outdated
@@ -17,6 +17,8 @@ | |||
|
|||
#include "namespaces.hh" | |||
|
|||
volatile sig_atomic_t RecursorControlChannel::stop; |
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.
Is the initial value set somewhere?
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.
https://en.cppreference.com/w/cpp/language/initialization defines that statics are zero initialized. But it might be better to do it explicitly indeed .
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!
This is safer since the atexit handler is not ran while threads are
still active. Also, when using valgrind we get more clean leak reports.
Retry of the accidentally merged #8518 that was reverted.
Short description
Checklist
I have: