coldtobi commented on a change in pull request #68:
URL: https://github.com/apache/logging-log4cxx/pull/68#discussion_r697873836
##########
File path: src/main/cpp/threadutility.cpp
##########
@@ -44,19 +50,23 @@ void ThreadUtility::configureFuncs( ThreadStartPre
pre_start,
void ThreadUtility::preThreadBlockSignals(){
#if LOG4CXX_HAS_PTHREAD_SIGMASK
+ m_priv->creation_mutex.lock();
Review comment:
> Thorsten's compiler doesn't support it,
(Thread local is required to be supported by compiler claiming C++11
compliance; and
https://logging.apache.org/log4cxx/latest_stable/dependencies.html says log4cxx
requires C++11 or later....)
I agree, as Signals is a Unix thing, ignoring it might be it as well.
Another (Unix) approach could be Thread-Specific-Data (pthread_key_create(),
pthread_setspecific() and friends)
Disclaimer: Never used those functions myself until now (as thread local
storage is far easier to use and has less overhead).
Thinking... Another idea would be around avoiding that singleton, or at
least using the singleton in a different way...
mmm....
Maybe a factory instead of the singleton? It could returns individual
IThreadFunctions-derived objects and thus having its own members variables
The factory would be configured by the Configure() function;
Rather than having individual functions to be set with configureFuncs() I'd
encapsulate the 3 functions in in the class/struct IThreadFunctions.
IThreadFunctions could have a (maybe static) clone() function to help the
factory.
(OTOH .. .I'm still not sure if this configurabiity/complixty is really
needed; Possibly it would be enough to block the signals unconditionally when
log4cxx starts a thread? I currently cant think of an usecase where a user
would not want that log4cxx just blocks the signals. (Please explain; I'm
obviously missing why you think it necessary)..)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
