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

logger: Use thread_local, move config into a function-level static variable #6639

Closed
wants to merge 3 commits into from

Conversation

rgacogne
Copy link
Member

Short description

This PR includes the commit from #6625 and additionally move the Logger configuration to a different struct kept inside a function-level static variable, making sure it's always properly initialized before being used.
The diff is not very large and should not be harder to maintain and use than the current version. It avoids relying on an undefined order of initialization that might prove a more serious issue in the future.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)

@tih
Copy link
Contributor

tih commented May 22, 2018

Tested on NetBSD, and verified to fix the crash I reported in #6624.

pdns/logger.cc Outdated

static Logger::Config& getLoggerConfig()
{
static Logger::Config config;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"magic" code should have a comment, if you ask me (:

@tih
Copy link
Contributor

tih commented May 29, 2018

As previously mentioned, this fixes my NetBSD/amd64 crash, and both the auth server and the recursor work (and log) fine with these patches installed. On NetBSD/arm, however (the 32 bit arm platform, to be specific), the auth server is fine, but the recursor crashes. The backtrace looks exactly like what I saw on NetBSD/amd64 without your patch, and it happens at the first logging attempt made by recursorThread(); the "Done priming cache with root hints" one.

Might there be something the recursor isn't doing during startup, that it ought to?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants