Logging refactoring continued #364
Conversation
|
This PR is complete. I have tested (Linux/Ubuntu) all combinations of |
|
The last fix needs more investigation. |
|
So the issue is quite complicated. At first it was whenever the OpenGL processor prints errors like this The The commit "Fix exception handling in LogMessage destructor" removed usage of I tried adding some The only other access to thread-local variables happens in According to https://bugzilla.redhat.com/show_bug.cgi?id=1219646, ocl-icd implementation was fundamentally flawed (before 2.2.4) in its use of thread local storage in the library constructor. If I disable OpenCL, or update ocl-icd-libopencl1 to 2.2.7-2 (Ubuntu 15.10 Wily Werewolf), the problem goes away. Other bug reports of the same issue: According to GCC docs, "No static initialization may refer to the address of a thread-local variable." It looks like the case in these reports. Now that many users are stuck with Ubuntu 14.04 with the old version of ocl-icd, a work around has to be provided. Suppose no further access to any thread local variable is allowed under the buggy ocl-icd. That means no access to To avoid It is harder to avoid exceptions. There are some other issues with OpenCL (ocl-icd version detection and NOEXCEPTION setup, CMake cleanup, macro detection, embedding a cl.hpp because newer opencl-headers no longer has one). In terms of PRs I'll probably address all OpenCL issues in a new one. |
|
The above analysis means the last fix is unneeded. For platforms without the buggy ocl-icd, std::cerr will work as expected instead of replacing it with stderr. The next step is to remove the last fix and add a commit of platform specific check for buggy ocl-icd. |
fixed WithLogImpl::setLog; removed global ConsoleLog instance; updated Freenect2 to manage lifetime of Log instance renamed Log to Logger added LIBFREENECT2_API macro to logging classes added environment variable LIBFREENECT2_LOGGER_LEVEL to change default logger level, possible values 'debug','info','warning','error' made logger level immutable
Before this commit, logger pointers get passed around through inheritance and manually constructed dependency assignment lists. The manual management is hard to scale with logging calls which can appear anywhere in the code. This commit implements a single global static logger for all Freenect2 contexts. It still can be replaced by different loggers, but only one at a time. Now it is the responsibility of each logging point to include libfreenect2/logging.h, which is not automatically included.
Also add a "None" logging level Thus remove NoopLogger, and sort logging levels by verbosity.
Also implement a WithPerfLogging class based on timing code to remove duplicate timing code in several processors.
Also export level2str() in Logger for external use.
ocl-icd under 2.2.3 calls dlopen() in its library constructor and accesses a thread local variable in the process. This causes all subsequent access to any other thread local variables to deadlock. The bug is fixed in ocl-icd 2.2.4, which is not in stable releases in Ubuntu or Debian. Thus this provides a workaround given buggy ocl-icd. To avoid access to thread local variable, errno, std::ostream with unitbuf, and exception handling in libstdc++ cannot be used. This commit checks ocl-icd version, and refactor the OpenCL processor to not use exceptions. Then disable unitbuf on std::cerr and disable all exceptions with -fno-exceptions (when available). This commit and the ocl-icd bug do not affect Mac OS X or Windows.
|
Rebased to master. |
|
Thanks for your work. I've tested this on my machine, and everything works fine, so let's merge it. |
Continuing from #309.
I changed the logging API to
libfreenect2::{get,set}GlobalLogger()(instead ofsetLogger()) in case in the future we want the non-global logger interface again. The rationale of reverting to a global logger is described in this commit "Convert to a global static logger" and in the discussion in #309.libfreenect2 still manages the memory of the logger passed in because it needs access to the logger until the very end in all kinds of destructors.