You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2022/01/17 07:54:14 UTC
[GitHub] [logging-log4cxx] swebb2066 opened a new pull request #105: Prevent static initialization order faults by using local statics
swebb2066 opened a new pull request #105:
URL: https://github.com/apache/logging-log4cxx/pull/105
This PR is further work towards fixing LOGCXX-532.
I believe localstatics are thread safe since C++11
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] rm5248 commented on pull request #105: Prevent static initialization order faults by using local statics
Posted by GitBox <gi...@apache.org>.
rm5248 commented on pull request #105:
URL: https://github.com/apache/logging-log4cxx/pull/105#issuecomment-1014550892
One (potential) issue that I see here is that because the variables are still declared as `static`, they may get destroyed when the application is exiting(but another thread is still logging). It seems to me that we should either do the construction in `APRInitializer` or somehow have the `APRInitializer` hold a copy of the pointer to ensure proper destruction(the stackoverflow link in LOGCXX-532 has some good details on this).
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] swebb2066 edited a comment on pull request #105: Prevent static initialization order faults by using local statics
Posted by GitBox <gi...@apache.org>.
swebb2066 edited a comment on pull request #105:
URL: https://github.com/apache/logging-log4cxx/pull/105#issuecomment-1015034924
If `static` objects are being destroyed while another thread is logging, then `APRInitializer` is also destroyed (which is bad). So all logging activity must be stopped in `Logger` before `apr_terminate()` is called. Removing the heirarchy from the `Logger` did this (except for threads active in doAppend()).
I am getting SegFault in multithreadtest on Ubuntu now with this PR. I am thinking the `const LevelPtr&` return values (i.e. no locking in `Level::getDebug()` ) in this PR expose the case where threads active in doAppend().
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] rm5248 commented on pull request #105: Prevent static initialization order faults by using local statics
Posted by GitBox <gi...@apache.org>.
rm5248 commented on pull request #105:
URL: https://github.com/apache/logging-log4cxx/pull/105#issuecomment-1015955472
> If `static` objects are being destroyed while another thread is logging, then `APRInitializer` is also destroyed (which is bad). So all logging activity must be stopped in `Logger` before `apr_terminate()` is called. Removing the heirarchy from the `Logger` did this (except for threads active in doAppend()).
>
This is the big problem that I've been trying to wrap my head around. What exists currently is the least bad thing in my mind, since if you unexpectedly exit(which you shouldn't do in the first place) you at least get some log messages after that, it mostly works.
> I am getting SegFault in multithreadtest on Ubuntu now with this PR. I am thinking the `const LevelPtr&` return values (i.e. no locking in `Level::getDebug()` ) in this PR expose the case where threads are active in doAppend().
It's been a while, but that does sound right to me. I know that what I did made segfaults happen _less_, but they're still not completely fixed. I suspect that a more complex situation would make it more apparent
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] rm5248 commented on pull request #105: Prevent static initialization order faults by using local statics
Posted by GitBox <gi...@apache.org>.
rm5248 commented on pull request #105:
URL: https://github.com/apache/logging-log4cxx/pull/105#issuecomment-1047304623
You could try using a vcpkg overlay to build a custom version of log4cxx. See the [vcpkg documentation](https://github.com/microsoft/vcpkg/blob/master/docs/specifications/ports-overlay.md) for more information on that. I think that you can just copy the [provided portfiles](https://github.com/microsoft/vcpkg/tree/master/ports/log4cxx) and point it at a specific tag/URL if you need to.
Ultimately this is a rather complicated thing to solve, and there's really not much we can do to fix it - applications need to make sure that things shutdown correctly. If you do have a specific test case though, that would be useful.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] swebb2066 merged pull request #105: Prevent static initialization order faults by using local statics
Posted by GitBox <gi...@apache.org>.
swebb2066 merged pull request #105:
URL: https://github.com/apache/logging-log4cxx/pull/105
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] swebb2066 commented on pull request #105: Prevent static initialization order faults by using local statics
Posted by GitBox <gi...@apache.org>.
swebb2066 commented on pull request #105:
URL: https://github.com/apache/logging-log4cxx/pull/105#issuecomment-1015034924
If `static` objects are being destroyed while another thread is logging, then `APRInitializer` is also destroyed (which is bad). So all logging activity must be stopped in `Logger` before . Removing the heirarchy from the `Logger` did this (except for threads active in append()).
We must inactivate all Loggers before `apr_terminate()` is called.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] LeeGDavis commented on pull request #105: Prevent static initialization order faults by using local statics
Posted by GitBox <gi...@apache.org>.
LeeGDavis commented on pull request #105:
URL: https://github.com/apache/logging-log4cxx/pull/105#issuecomment-1047317100
Thanks @rm5248. I'll give that a try. I'll see what I can do about a test case too.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] LeeGDavis commented on pull request #105: Prevent static initialization order faults by using local statics
Posted by GitBox <gi...@apache.org>.
LeeGDavis commented on pull request #105:
URL: https://github.com/apache/logging-log4cxx/pull/105#issuecomment-1047299349
I think we're running into this issue after and upgrade to 0.12.1 from 0.11.0. Is there any way to tests this or what is currently in [apache:next_stable](https://github.com/apache/logging-log4cxx/tree/next_stable) using vcpkg? Happy to contribute any feedback.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] swebb2066 edited a comment on pull request #105: Prevent static initialization order faults by using local statics
Posted by GitBox <gi...@apache.org>.
swebb2066 edited a comment on pull request #105:
URL: https://github.com/apache/logging-log4cxx/pull/105#issuecomment-1015034924
If `static` objects are being destroyed while another thread is logging, then `APRInitializer` is also destroyed (which is bad). So all logging activity must be stopped in `Logger` before `apr_terminate()` is called. Removing the heirarchy from the `Logger` did this (except for threads active in doAppend()).
I am getting SegFault in multithreadtest on Ubuntu now with this PR. I am thinking the `const LevelPtr&` return values (i.e. no locking in `Level::getDebug()` ) in this PR expose the case where threads are active in doAppend().
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org