[libcamera-devel] [RFC PATCH] libcamera: base: Make the registration of log categories atomic
Nicolas Dufresne
nicolas at ndufresne.ca
Tue Mar 14 14:00:46 CET 2023
Le mardi 14 mars 2023 à 14:50 +0530, Umang Jain a écrit :
> Hi Nicolas
>
> On 3/14/23 2:11 AM, Nicolas Dufresne via libcamera-devel wrote:
> > From: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> >
> > Logger:::create() is not currently thread safe and causes crashes
Spotting some of my own typos now, should have two :: here.
>
> Indeed. Given the fact that gstlibcamerasrc should be ideally moving
> away from using libcamera_private dependency (as LogCategory::create())
> is an internal API
gstlibcamerasrc is using GStreamer logger only. Perhaps I'm missing something ?
You'll see in the backtrace that libgstcamerasrc is not involved.
>
> I can see that it's being called from other internal threads as well
> (like PostProcessorWorker from android HAL layer) so making it thread
> safe makes sense.
Indeed, adding some sort of safety, but resorting to a global lock might not be
the best solution, that's why I started with an RFC.
>
> > noticeable on RaspberryPi 4. This adds a mutex around the creation
> > of categories.
> >
> > Signed-off-by: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> > ---
> >
> > src/libcamera/base/log.cpp | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > This is a quick fix. The crash I'm getting is very frequent, the backtrace
> > is the following. Pretty much 50% of the time on Raspberry Pi using:
> >
> > gst-launch-1.0 libcamerasrc ! queue ! xvimagesink
> >
> > Thread 6 "gst-launch-1.0" received signal SIGSEGV, Segmentation fault.
> > [Switching to Thread 0x7ff48cc180 (LWP 88100)]
> > 0x0000007ff5f48a40 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::compare(char const*) const () from /lib/aarch64-linux-gnu/libstdc++.so.6
> > (gdb) bt
> > #0 0x0000007ff5f48a40 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::compare(char const*) const () at /lib/aarch64-linux-gnu/libstdc++.so.6
> > #1 0x0000007ff664efe0 in std::operator==<char, std::char_traits<char>, std::allocator<char> >(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, char const*)
> > (__lhs=<error reading variable: Cannot access memory at address 0x705f317073696b7a>, __rhs=0x7ff61c4a28 "Event") at /usr/include/c++/10/bits/basic_string.h:6187
> > #2 0x0000007ff61b48d4 in operator()<libcamera::LogCategory*>(libcamera::LogCategory*) const (__closure=0x7ff48cb008, c=0x705f317073696b72) at ../src/libcamera/base/log.cpp:748
> > #3 0x0000007ff61b4910 in __gnu_cxx::__ops::_Iter_pred<libcamera::Logger::findCategory(char const*) const::<lambda(auto:2)> >::operator()<__gnu_cxx::__normal_iterator<libcamera::LogCategory* const*, std::vector<libcamera::LogCategory*> > >(__gnu_cxx::__normal_iterator<libcamera::LogCategory* const*, std::vector<libcamera::LogCategory*, std::allocator<libcamera::LogCategory*> > >)
> > (this=0x7ff48cb008, __it=0x705f317073696b72) at /usr/include/c++/10/bits/predefined_ops.h:316
> > #4 0x0000007ff61b487c in std::__find_if<__gnu_cxx::__normal_iterator<libcamera::LogCategory* const*, std::vector<libcamera::LogCategory*> >, __gnu_cxx::__ops::_Iter_pred<libcamera::Logger::findCategory(char const*) const::<lambda(auto:2)> > >(__gnu_cxx::__normal_iterator<libcamera::LogCategory* const*, std::vector<libcamera::LogCategory*, std::allocator<libcamera::LogCategory*> > >, __gnu_cxx::__normal_iterator<libcamera::LogCategory* const*, std::vector<libcamera::LogCategory*, std::allocator<libcamera::LogCategory*> > >, __gnu_cxx::__ops::_Iter_pred<libcamera::Logger::findCategory(char const*) const::<lambda(auto:2)> >, std::random_access_iterator_tag) (__first=0x705f317073696b72, __last=0x303100736d617261, __pred=...) at /usr/include/c++/10/bits/stl_algobase.h:1959
> > #5 0x0000007ff61b46b4 in std::__find_if<__gnu_cxx::__normal_iterator<libcamera::LogCategory* const*, std::vector<libcamera::LogCategory*> >, __gnu_cxx::__ops::_Iter_pred<libcamera::Logger::findCategory(char const*) const::<lambda(auto:2)> > >(__gnu_cxx::__normal_iterator<libcamera::LogCategory* const*, std::vector<libcamera::LogCategory*, std::allocator<libcamera::LogCategory*> > >, __gnu_cxx::__normal_iterator<libcamera::LogCategory* const*, std::vector<libcamera::LogCategory*, std::allocator<libcamera::LogCategory*> > >, __gnu_cxx::__ops::_Iter_pred<libcamera::Logger::findCategory(char const*) const::<lambda(auto:2)> >)
> > (__first=0x555582c130, __last=0x303100736d617261, __pred=...) at /usr/include/c++/10/bits/stl_algobase.h:1974
> > #6 0x0000007ff61b4648 in std::find_if<__gnu_cxx::__normal_iterator<libcamera::LogCategory* const*, std::vector<libcamera::LogCategory*> >, libcamera::Logger::findCategory(char const*) const::<lambda(auto:2)> >(__gnu_cxx::__normal_iterator<libcamera::LogCategory* const*, std::vector<libcamera::LogCategory*, std::allocator<libcamera::LogCategory*> > >, __gnu_cxx::__normal_iterator<libcamera::LogCategory* const*, std::vector<libcamera::LogCategory*, std::allocator<libcamera::LogCategory*> > >, struct {...}) (__first=0x555582c130, __last=0x303100736d617261, __pred=...) at /usr/include/c++/10/bits/stl_algo.h:3929
> > #7 0x0000007ff61b3f60 in libcamera::Logger::findCategory(char const*) const (this=0x7ff61ec968 <libcamera::Logger::instance()::instance>, name=0x7ff61c4a28 "Event") at ../src/libcamera/base/log.cpp:748
> > #8 0x0000007ff61b3fc4 in libcamera::LogCategory::create(char const*) (name=0x7ff61c4a28 "Event") at ../src/libcamera/base/log.cpp:791
> > #9 0x0000007ff61a9ffc in libcamera::logCategoryEvent() () at ../src/libcamera/base/event_dispatcher.cpp:17
> > #10 0x0000007ff61aac5c in libcamera::EventDispatcherPoll::poll(std::vector<pollfd, std::allocator<pollfd> >*) (this=0x7fe8006f90, pollfds=0x7ff48cb3e8) at ../src/libcamera/base/event_dispatcher_poll.cpp:216
> > #11 0x0000007ff61aa8fc in libcamera::EventDispatcherPoll::processEvents() (this=0x7fe8006f90) at ../src/libcamera/base/event_dispatcher_poll.cpp:160
> > #12 0x0000007ff61be0bc in libcamera::Thread::exec() (this=0x55555d0380) at ../src/libcamera/base/thread.cpp:341
> > #13 0x0000007ff6643e88 in libcamera::CameraManager::Private::run() (this=0x55555d0370) at ../src/libcamera/camera_manager.cpp:122
> > #14 0x0000007ff61be038 in libcamera::Thread::startThread() (this=0x55555d0380) at ../src/libcamera/base/thread.cpp:319
> > #15 0x0000007ff61c2070 in std::__invoke_impl<void, void (libcamera::Thread::*)(), libcamera::Thread*>(std::__invoke_memfun_deref, void (libcamera::Thread::*&&)(), libcamera::Thread*&&)
> > (__f=@0x5555852ef0: (void (libcamera::Thread::*)(class libcamera::Thread * const)) 0x7ff61bdf38 <libcamera::Thread::startThread()>, __t=@0x5555852ee8: 0x55555d0380) at /usr/include/c++/10/bits/invoke.h:73
> > #16 0x0000007ff61c1f88 in std::__invoke<void (libcamera::Thread::*)(), libcamera::Thread*>(void (libcamera::Thread::*&&)(), libcamera::Thread*&&)
> > (__fn=@0x5555852ef0: (void (libcamera::Thread::*)(class libcamera::Thread * const)) 0x7ff61bdf38 <libcamera::Thread::startThread()>) at /usr/include/c++/10/bits/invoke.h:95
> > #17 0x0000007ff61c1eec in std::thread::_Invoker<std::tuple<void (libcamera::Thread::*)(), libcamera::Thread*> >::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) (this=0x5555852ee8)
> > at /usr/include/c++/10/thread:264
> > #18 0x0000007ff61c1ea4 in std::thread::_Invoker<std::tuple<void (libcamera::Thread::*)(), libcamera::Thread*> >::operator()() (this=0x5555852ee8) at /usr/include/c++/10/thread:271
> > #19 0x0000007ff61c1e84 in std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (libcamera::Thread::*)(), libcamera::Thread*> > >::_M_run() (this=0x5555852ee0) at /usr/include/c++/10/thread:215
> > #20 0x0000007ff5ee1cac in () at /lib/aarch64-linux-gnu/libstdc++.so.6
> > #21 0x0000007ff7c78648 in start_thread (arg=0x7ff48cba80) at pthread_create.c:477
> > #22 0x0000007ff7bcec1c in thread_start () at ../sysdeps/unix/sysv/linux/aarch64/clone.S:78
> >
> >
> > diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp
> > index 55fbd7b0..c8045ef7 100644
> > --- a/src/libcamera/base/log.cpp
> > +++ b/src/libcamera/base/log.cpp
> > @@ -21,6 +21,7 @@
> > #include <libcamera/logging.h>
> >
> > #include <libcamera/base/backtrace.h>
> > +#include <libcamera/base/mutex.h>
> > #include <libcamera/base/thread.h>
> > #include <libcamera/base/utils.h>
> >
> > @@ -788,6 +789,8 @@ LogCategory *Logger::findCategory(const char *name) const
> > */
> > LogCategory *LogCategory::create(const char *name)
> > {
> > + static Mutex mutex_;
> > + MutexLocker locker(mutex_);
> > LogCategory *category = Logger::instance()->findCategory(name);
> >
> > if (!category) {
>
More information about the libcamera-devel
mailing list