Wrapping global verbosity variable in loggic with atomic to ensure threadsafety.

[ghost]

This came up when running xgboost with a thread sanitizer. It looks like a couple of code paths repeatedly call ConsoleLogger::Configure. The result is two different threads might be setting global_verbosity_. Atomic seemed like the most straightforward way to fix the issue without refactoring a more things.

[trivialfis]

Most of the APIs are not thread safe.

[trivialfis]

If we were to guarantee the thread safety for all API functions, we should work on the learner class. But I can't find a use case for it except for prediction.

[ghost]

Yeah it's understandable the APIs are not thread safe. One issue here is that since it's editing a global variable it cannot be made thread safe. I am running the xgboost learner code in a separate thread where all allocations and access are done on that thread so all those apis don't have thread safety concerns. I thought this was at least worth making a PR for, but please feel free to close if it's a non-issue or outside the scope of support.

[codecov-commenter]

Codecov Report

Merging #5947 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #5947   +/-   ##
=======================================
  Coverage   78.11%   78.11%           
=======================================
  Files          12       12           
  Lines        2979     2979           
=======================================
  Hits         2327     2327           
  Misses        652      652           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8943eb4...162da0b. Read the comment docs.

[trivialfis]

Thanks for bringing this up. Will take another look after #5853 being sorted out.

[hcho3]

I'm not sure if the logging level should be thread-safe. Will the logging function misbehave if it gets a stale value of the logging level? At worst we may see a few lines of logs that should not have been printed.

[ghost]

True, that's a good point. I think writing an int32_t across threads is technically undefined but on x86/x86-64 it should be fine and I don't think anyone is really running xgboost on obscure archs. I am going to close this since it seems probably fine.