i2pd icon indicating copy to clipboard operation
i2pd copied to clipboard

Intermittent division by zero error in Transports::GetRandomPeer()

Open chadf opened this issue 2 years ago • 2 comments

Transports::GetRandomPeer() randomly fails due to division by zero.

==38082==ERROR: AddressSanitizer: FPE on unknown address 0x00000186426a (pc 0x00000186426a bp 0x7fffdf3f4cf0 sp 0x7fffdf3f44a0 T7)
    #0 0x186426a in std::__1::shared_ptr<i2p::data::RouterInfo const> i2p::transport::Transports::GetRandomPeer<i2p::transport::Transports::GetRandomPeer(bool) const::$_3>(i2p::transport::Transports::GetRandomPeer(bool) const::$_3) const /projects/i2pd/libi2pd/Transports.cpp:864:12
    #1 0x1863af6 in i2p::transport::Transports::GetRandomPeer(bool) const /projects/i2pd/libi2pd/Transports.cpp:941:10
    #2 0x20002c9 in i2p::tunnel::TunnelPool::StandardSelectPeers(i2p::tunnel::Path&, int, bool, std::__1::function<std::__1::shared_ptr<i2p::data::RouterInfo const> (std::__1::shared_ptr<i2p::data::RouterInfo const>, bool)>) /projects/i2pd/libi2pd/TunnelPool.cpp:515:38
    #3 0x2002859 in i2p::tunnel::TunnelPool::SelectPeers(i2p::tunnel::Path&, bool) /projects/i2pd/libi2pd/TunnelPool.cpp:569:10
    #4 0x1ff0d06 in i2p::tunnel::TunnelPool::CreateOutboundTunnel() /projects/i2pd/libi2pd/TunnelPool.cpp:661:7
    #5 0x1fef524 in i2p::tunnel::TunnelPool::CreateTunnels() /projects/i2pd/libi2pd/TunnelPool.cpp:290:5
    #6 0x1ff91fe in i2p::tunnel::TunnelPool::ManageTunnels(unsigned long) /projects/i2pd/libi2pd/TunnelPool.cpp:397:4
    #7 0x1e8918d in i2p::tunnel::Tunnels::ManageTunnelPools(unsigned long) /projects/i2pd/libi2pd/Tunnel.cpp:822:11
    #8 0x1e8333f in i2p::tunnel::Tunnels::Run() /projects/i2pd/libi2pd/Tunnel.cpp:550:7
    #9 0x1f12fcd in decltype(*(static_cast<i2p::tunnel::Tunnels*&>(fp0)).*fp()) std::__1::__invoke<void (i2p::tunnel::Tunnels::*&)(), i2p::tunnel::Tunnels*&, void>(void (i2p::tunnel::Tunnels::*&)(), i2p::tunnel::Tunnels*&) /usr/include/c++/v1/type_traits:3565:23
    #10 0x1f12e0d in std::__1::__bind_return<void (i2p::tunnel::Tunnels::*)(), std::__1::tuple<i2p::tunnel::Tunnels*>, std::__1::tuple<>, __is_valid_bind_return<void (i2p::tunnel::Tunnels::*)(), std::__1::tuple<i2p::tunnel::Tunnels*>, std::__1::tuple<> >::value>::type std::__1::__apply_functor<void (i2p::tunnel::Tunnels::*)(), std::__1::tuple<i2p::tunnel::Tunnels*>, 0ul, std::__1::tuple<> >(void (i2p::tunnel::Tunnels::*&)(), std::__1::tuple<i2p::tunnel::Tunnels*>&, std::__1::__tuple_indices<0ul>, std::__1::tuple<>&&) /usr/include/c++/v1/__functional/bind.h:263:12
    #11 0x1f12b54 in std::__1::__bind_return<void (i2p::tunnel::Tunnels::*)(), std::__1::tuple<i2p::tunnel::Tunnels*>, std::__1::tuple<>, __is_valid_bind_return<void (i2p::tunnel::Tunnels::*)(), std::__1::tuple<i2p::tunnel::Tunnels*>, std::__1::tuple<> >::value>::type std::__1::__bind<void (i2p::tunnel::Tunnels::*)(), i2p::tunnel::Tunnels*>::operator()<>() /usr/include/c++/v1/__functional/bind.h:298:20
    #12 0x1f126db in decltype(static_cast<std::__1::__bind<void (i2p::tunnel::Tunnels::*)(), i2p::tunnel::Tunnels*>>(fp)()) std::__1::__invoke<std::__1::__bind<void (i2p::tunnel::Tunnels::*)(), i2p::tunnel::Tunnels*> >(std::__1::__bind<void (i2p::tunnel::Tunnels::*)(), i2p::tunnel::Tunnels*>&&) /usr/include/c++/v1/type_traits:3640:23
    #13 0x1f1262d in void std::__1::__thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, std::__1::__bind<void (i2p::tunnel::Tunnels::*)(), i2p::tunnel::Tunnels*> >(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, std::__1::__bind<void (i2p::tunnel::Tunnels::*)(), i2p::tunnel::Tunnels*> >&, std::__1::__tuple_indices<>) /usr/include/c++/v1/thread:282:5
    #14 0x1f11244 in void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, std::__1::__bind<void (i2p::tunnel::Tunnels::*)(), i2p::tunnel::Tunnels*> > >(void*) /usr/include/c++/v1/thread:293:5
    #15 0x80271ea79 in thread_start /usr/src/lib/libthr/thread/thr_create.c:292:16

I suspect it is a race condition between the m_Peers.empty() check and m_Peers.size() call, where another thread (e.g. Transports::Run()) is removing elements.

If elements are not immutable after adding, there is also the potential one thread could be using a RouterInfo while another is updating it, resulting in inconsistent data and/or corruption.

chadf avatar Jul 06 '23 00:07 chadf

i2pd have many problems with thread safety, yeah. My issue #1935 for example have similar origins probably. What you found is important, thank you. This place must be designed to be thread safe. But there is one place, where there were even no attempts to make it thread safe: web console. So don't be surprised by it. Web console problems deserves its own bug report, but who and when will fix it is a big mystery. Historically, web console was made for debugging purposes. Now it is used by users, but architecture was not changed since then.

Vort avatar Jul 06 '23 06:07 Vort

After looking at it again, I did notice it is getting a mutex lock before the divide, but not before the empty check. By either moving the first check to after the mutex, or add a second check (to avoid RAND_bytes() overhead if the queue is empty most of the time), it could fix the division by zero issue.

There is still the potential of other thread issues, but at least it would be one less.

chadf avatar Jul 06 '23 23:07 chadf