| Message ID | 20230313204134.2769860-1-nicolas@ndufresne.ca |
|---|---|
| State | Accepted |
| Commit | 2ccca099d3cbdfa32f0a8bfd2168873b7b803159 |
| Headers | show |
| Series |
|
| Related | show |
Hi Nicolas On 3/14/23 2:11 AM, Nicolas Dufresne via libcamera-devel wrote: > From: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > Logger:::create() is not currently thread safe and causes crashes Indeed. Given the fact that gstlibcamerasrc should be ideally moving away from using libcamera_private dependency (as LogCategory::create()) is an internal API 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. > noticeable on RaspberryPi 4. This adds a mutex around the creation > of categories. > > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@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) {
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@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@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) { >
Quoting Nicolas Dufresne via libcamera-devel (2023-03-14 13:00:46) > 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@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. A cursory look through, I don't think this is a problem. This is likely as fine-grained as you can get. I expect the s/:::/::/ can be fixed when applying. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > > noticeable on RaspberryPi 4. This adds a mutex around the creation > > > of categories. > > > > > > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@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) { > > >
On Tue, Mar 21, 2023 at 05:44:38PM +0000, Kieran Bingham via libcamera-devel wrote: > Quoting Nicolas Dufresne via libcamera-devel (2023-03-14 13:00:46) > > 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@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. > > A cursory look through, I don't think this is a problem. This is likely > as fine-grained as you can get. > > I expect the s/:::/::/ can be fixed when applying. > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> The create() function is only called when log categories are created, which is on first use. The lock contention shouldn't be a big deal, it's good enough for now. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > noticeable on RaspberryPi 4. This adds a mutex around the creation > > > > of categories. > > > > > > > > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@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) { > > > > >
Le mardi 21 mars 2023 à 19:50 +0200, Laurent Pinchart a écrit : > On Tue, Mar 21, 2023 at 05:44:38PM +0000, Kieran Bingham via libcamera-devel wrote: > > Quoting Nicolas Dufresne via libcamera-devel (2023-03-14 13:00:46) > > > 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@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. > > > > A cursory look through, I don't think this is a problem. This is likely > > as fine-grained as you can get. > > > > I expect the s/:::/::/ can be fixed when applying. > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > The create() function is only called when log categories are created, > which is on first use. The lock contention shouldn't be a big deal, it's > good enough for now. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Thanks for the reviews, so feel free to fix my tripped : typo and drop the RFC. regards, Nicolas > > > > > > noticeable on RaspberryPi 4. This adds a mutex around the creation > > > > > of categories. > > > > > > > > > > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@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) { > > > > > > > >
Hi Nicolas, Quoting Nicolas Dufresne (2023-03-22 15:43:17) > Le mardi 21 mars 2023 à 19:50 +0200, Laurent Pinchart a écrit : > > On Tue, Mar 21, 2023 at 05:44:38PM +0000, Kieran Bingham via libcamera-devel wrote: > > > Quoting Nicolas Dufresne via libcamera-devel (2023-03-14 13:00:46) > > > > > > I expect the s/:::/::/ can be fixed when applying. > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Thanks for the reviews, so feel free to fix my tripped : typo and drop the RFC. No worries. This was merged last night: - https://git.libcamera.org/libcamera/libcamera.git/commit/?id=2ccca099d3cbdfa32f0a8bfd2168873b7b803159 -- Kieran
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) {