cicwi / SliceRecon

This repository has moved to:
https://cicwi.github.io/RECAST3D
GNU General Public License v3.0
6 stars 4 forks source link

Logging thread safety #13

Closed wjp closed 4 years ago

wjp commented 4 years ago

This crash is presumably caused by the logger not being thread-safe:

#0  0x00007ffff6e918e6 in __memmove_avx_unaligned_erms () from /lib64/libc.so.6
#1  0x00007ffff7f194a8 in std::basic_streambuf<char, std::char_traits<char> >::xsputn(char const*, long) () from /lib64/libstdc++.so.6
#2  0x00007ffff7f08083 in std::basic_ostream<char, std::char_traits<char> >& std::__ostream_insert<char, std::char_traits<char> >(std::basic_ostream<char, std::char_traits<char> >&, char const*, long) () from /lib64/libstdc++.so.6
#3  0x000000000043750f in std::operator<< <std::char_traits<char> > (__out=..., __s=0x444d55 "[") at /usr/include/c++/9/bits/char_traits.h:335
#4  slicerecon::util::logger::operator<< <char const (&) [2]> (rhs=..., this=<optimized out>)
    at /export/scratch1/home/wjp/build/SliceRecon/include/slicerecon/reconstruction/../util/log.hpp:43
#5  slicerecon::detail::cone_beam_solver::reconstruct_slice (this=0x7fffe00082e0, x=..., buffer_idx=0)
    at /export/scratch1/home/wjp/build/SliceRecon/src/reconstruction/reconstructor.cpp:363
#6  0x00000000004187bc in slicerecon::reconstructor::reconstruct_slice (x=..., this=0x4d9940) at /usr/include/c++/9/bits/unique_ptr.h:357
#7  <lambda(auto:38, auto:39)>::operator()<std::array<float, 9>, int> (__closure=<optimized out>, idx=<optimized out>, x=...)
    at /export/scratch1/home/wjp/build/SliceRecon/example/slicerecon_server.cpp:88
#8  std::_Function_handler<std::pair<std::array<int, 2>, std::vector<float, std::allocator<float> > >(std::array<float, 9>, int), main(int, char**)::<lambda(auto:38, auto:39)> >::_M_invoke(const std::_Any_data &, std::array<float, 9> &&, int &&) (__functor=..., __args#0=..., 
    __args#1=<optimized out>) at /usr/include/c++/9/bits/std_function.h:286
#9  0x000000000042b121 in std::function<std::pair<std::array<int, 2ul>, std::vector<float, std::allocator<float> > > (std::array<float, 9ul>, int)>::operator()(std::array<float, 9ul>, int) const (__args#1=<optimized out>, __args#0=..., this=0x7fffffffcdb8)
    at /usr/include/c++/9/bits/std_function.h:685
#10 slicerecon::visualization_server::make_slice (orientation=..., slice_id=222, this=0x7fffffffcd50)
    at /export/scratch1/home/wjp/build/SliceRecon/include/slicerecon/servers/visualization_server.hpp:290
#11 slicerecon::visualization_server::serve()::{lambda()#1}::operator()() const (__closure=0x50b638)
    at /export/scratch1/home/wjp/build/SliceRecon/include/slicerecon/servers/visualization_server.hpp:188
#12 0x00007ffff7ea96f4 in ?? () from /lib64/libstdc++.so.6
#13 0x00007ffff72704c0 in start_thread () from /lib64/libpthread.so.0
#14 0x00007ffff6e2e553 in clone () from /lib64/libc.so.6

A quick fix would probably be:

index b92c156..fd67c5a 100644
@@ -40,11 +40,15 @@ struct logger {

     template <typename T>
     logger& operator<<(T&& rhs) {
+        std::lock_guard lock(mutex);
+
         line << rhs;
         return *this;
     }

     void operator<<(end_log_ unused) {
+        std::lock_guard lock(mutex);
+
         (void)unused;

         switch (level_) {
@@ -69,6 +73,7 @@ struct logger {
         line = std::stringstream{};
     }

+    std::mutex mutex;
     std::stringstream line;
     lvl level_ = lvl::none_set;
 };

However, that wouldn't make logging a single line atomic. Any opinions on if this quick fix would be sufficient?

jwbuurlage commented 4 years ago

Yeah, I think guarding the global logger with a mutex is the way to go. This shouldn't impact performance too much, so feel free to add the lock.

wjp commented 4 years ago

Committed as 8e6bdaa51d2fbb2146276426b6405c7e89582484.