search

PurpleI2P / i2pd

🛡 I2P: End-to-End encrypted and anonymous Internet
https://i2pd.website
BSD 3-Clause "New" or "Revised" License
3.31k stars 424 forks source link

Assert while removing SAM socket, concurency & thread safety problem #207

Closed mlt closed 9 years ago

mlt commented 9 years ago

Assertion failed for a sockets list item while handling stream error. From what I see erase found an item, but when it tried to delete it an assert failed that block was not in use. I see that an iterator is not valid in list::remove .

orignal commented 9 years ago

I think session socket doesn't exist anymore by that moment, but functor of HandleI2PReceieve still holds pointer to SAMSocket together with corresponding stream since it has to close it properly. Let me think how can it be fixed.

mlt commented 9 years ago

uhm... there is a loop in list::remove. It got to the point where it found an iterator matching to the passed in value. I suspect that sockets list (why not set?) shall be guarded by mutex. And even better make it private by exposing add/remove functions.

orignal commented 9 years ago

I agree, that's race condition. "Teminate ()" from line 593 should be posted to SAMBridge's thread by calling "post" method io_service from socket. You wouldn't need a mutex in this case.

mlt commented 9 years ago

Do you mean like m_Owner.GetService().post(boost::bind(&SAMSocket::Terminate, shared_from_this ())); ? This works so far. What about CloseStream inside of Terminate? And there are a few places like that in SAM.cpp . Shall Terminate be split into 2 functions, i.e. CloseStream & posting the rest to SAM bridge?

orignal commented 9 years ago

Yes. Or even better if you take io_service from socket. Also don't forget to do the same when you call Stream::AsyncWrite. That's good idea to split termination of stream and socket. Close stream wouldn't produce an infinite loop, because it returns operation_aborted

mlt commented 9 years ago

Also I just got access violation at 0x0000001f. I'm not sure about details. It doesn't make any sense to me what went wrong. It has only the change I mentioned without splitting Terminate etc. I saved the core dump just in case.

    i2pd-bin.exe!std::_Ref_count_base::_Decref() Line 118   C++
    i2pd-bin.exe!std::_Ptr_base<i2p::client::SAMSocket>::_Decref() Line 344 C++
>   i2pd-bin.exe!std::shared_ptr<i2p::client::SAMSocket>::~shared_ptr<i2p::client::SAMSocket>() Line 611    C++
    i2pd-bin.exe!std::_Tuple_val<std::shared_ptr<i2p::client::SAMSocket> >::~_Tuple_val<std::shared_ptr<i2p::client::SAMSocket> >() C++
    i2pd-bin.exe!std::tuple<std::shared_ptr<i2p::client::SAMSocket>,std::_Ph<1>,std::_Ph<2> >::~tuple<std::shared_ptr<i2p::client::SAMSocket>,std::_Ph<1>,std::_Ph<2> >()   C++
    i2pd-bin.exe!std::_Bind<1,void,std::_Pmf_wrap<void (__thiscall i2p::client::SAMSocket::*)(boost::system::error_code const &,unsigned int),void,i2p::client::SAMSocket,boost::system::error_code const &,unsigned int>,std::shared_ptr<i2p::client::SAMSocket>,std::_Ph<1> &,std::_Ph<2> &>::~_Bind<1,void,std::_Pmf_wrap<void (__thiscall i2p::client::SAMSocket::*)(boost::system::error_code const &,unsigned int),void,i2p::client::SAMSocket,boost::system::error_code const &,unsigned int>,std::shared_ptr<i2p::client::SAMSocket>,std::_Ph<1> &,std::_Ph<2> &>() C++
    i2pd-bin.exe!void <lambda>(void)::~void <lambda>(void)()    C++
    i2pd-bin.exe!boost::asio::detail::completion_handler<void <lambda>(void) >::~completion_handler<void <lambda>(void) >() C++
    i2pd-bin.exe!boost::asio::detail::completion_handler<void <lambda>(void) >::`scalar deleting destructor'(unsigned int)  C++
    i2pd-bin.exe!boost::asio::detail::completion_handler<void <lambda>(void) >::ptr::reset() Line 35    C++
    i2pd-bin.exe!boost::asio::detail::completion_handler<void <lambda>(void) >::do_complete(boost::asio::detail::win_iocp_io_service * owner, boost::asio::detail::win_iocp_operation * base, const boost::system::error_code & __formal, unsigned int __formal) Line 64    C++
    i2pd-bin.exe!boost::asio::detail::win_iocp_operation::complete(boost::asio::detail::win_iocp_io_service & owner, const boost::system::error_code & ec, unsigned int bytes_transferred) Line 46  C++
    i2pd-bin.exe!boost::asio::detail::win_iocp_io_service::do_one(bool block, boost::system::error_code & ec) Line 406  C++
    i2pd-bin.exe!boost::asio::detail::win_iocp_io_service::run(boost::system::error_code & ec) Line 164 C++
    i2pd-bin.exe!boost::asio::io_service::run() Line 59 C++
    i2pd-bin.exe!i2p::client::ClientDestination::Run() Line 113 C++
    i2pd-bin.exe!std::_Pmf_wrap<void (__thiscall i2p::client::ClientDestination::*)(void),void,i2p::client::ClientDestination>::operator()(i2p::client::ClientDestination * _Pfnobj) Line 1230  C++
    i2pd-bin.exe!std::_Bind<1,void,std::_Pmf_wrap<void (__thiscall i2p::client::ClientDestination::*)(void),void,i2p::client::ClientDestination>,i2p::client::ClientDestination * const>::_Do_call<,0>(std::tuple<> _Myfargs, std::_Arg_idx<0> __formal) Line 1150  C++
    i2pd-bin.exe!std::_Bind<1,void,std::_Pmf_wrap<void (__thiscall i2p::client::ClientDestination::*)(void),void,i2p::client::ClientDestination>,i2p::client::ClientDestination * const>::operator()<>() Line 1138  C++
    i2pd-bin.exe!std::_Bind<0,void,std::_Bind<1,void,std::_Pmf_wrap<void (__thiscall i2p::client::ClientDestination::*)(void),void,i2p::client::ClientDestination>,i2p::client::ClientDestination * const> >::_Do_call<>(std::tuple<> _Myfargs, std::_Arg_idx<> __formal) Line 1150 C++
    i2pd-bin.exe!std::_Bind<0,void,std::_Bind<1,void,std::_Pmf_wrap<void (__thiscall i2p::client::ClientDestination::*)(void),void,i2p::client::ClientDestination>,i2p::client::ClientDestination * const> >::operator()<>() Line 1138  C++
    i2pd-bin.exe!std::_LaunchPad<std::_Bind<0,void,std::_Bind<1,void,std::_Pmf_wrap<void (__thiscall i2p::client::ClientDestination::*)(void),void,i2p::client::ClientDestination>,i2p::client::ClientDestination * const> > >::_Run(std::_LaunchPad<std::_Bind<0,void,std::_Bind<1,void,std::_Pmf_wrap<void (__thiscall i2p::client::ClientDestination::*)(void),void,i2p::client::ClientDestination>,i2p::client::ClientDestination * const> > > * _Ln) Line 196  C++
    i2pd-bin.exe!std::_LaunchPad<std::_Bind<0,void,std::_Bind<1,void,std::_Pmf_wrap<void (__thiscall i2p::client::ClientDestination::*)(void),void,i2p::client::ClientDestination>,i2p::client::ClientDestination * const> > >::_Go() Line 187  C++
    msvcp120d.dll!_Call_func(void * _Data) Line 28  C++
    msvcr120d.dll!_callthreadstartex() Line 376 C
    msvcr120d.dll!_threadstartex(void * ptd) Line 359   C
    kernel32.dll!@BaseThreadInitThunk@12() Unknown
    ntdll.dll!__RtlUserThreadStart()    Unknown
    ntdll.dll!__RtlUserThreadStart@8() Unknown
mlt commented 9 years ago

I'd like to solicit some feedback. I'm getting numerous bad_weak_ptr exceptions with this one for SAM bridge and ClientDestination. Though in general it works so far. I suspect it is in shared_from_this while erasing SAMSocket from set.

orignal commented 9 years ago

I had this issue before

mlt commented 9 years ago

With this patch it died on me

#0  0xb7738424 in __kernel_vsyscall ()
#1  0xb6e1a607 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
#2  0xb6e1db04 in __GI_abort () at abort.c:118
#3  0xb702bbb5 in __gnu_cxx::__verbose_terminate_handler() () from /usr/lib/i386-linux-gnu/libstdc++.so.6
#4  0xb70298d3 in ?? () from /usr/lib/i386-linux-gnu/libstdc++.so.6
#5  0xb7028749 in ?? () from /usr/lib/i386-linux-gnu/libstdc++.so.6
#6  0xb7028ebe in __gxx_personality_v0 () from /usr/lib/i386-linux-gnu/libstdc++.so.6
#7  0xb6fafd2f in ?? () from /lib/i386-linux-gnu/libgcc_s.so.1
#8  0xb6fb008d in _Unwind_RaiseException () from /lib/i386-linux-gnu/libgcc_s.so.1
#9  0xb7029c04 in __cxa_throw () from /usr/lib/i386-linux-gnu/libstdc++.so.6
#10 0x0867af98 in std::__throw_bad_weak_ptr () at /usr/include/c++/4.8/bits/shared_ptr_base.h:76
#11 0x0867afc5 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_add_ref_lock (this=0xb3f2caf0) at /usr/include/c++/4.8/bits/shared_ptr_base.h:240
#12 0x0868f25a in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count (this=0xadcf7d2c, __r=...) at /usr/include/c++/4.8/bits/shared_ptr_base.h:699
#13 0x0873415e in std::__shared_ptr<i2p::client::SAMSocket, (__gnu_cxx::_Lock_policy)2>::__shared_ptr<i2p::client::SAMSocket> (this=0xadcf7d28, __r=...) at /usr/include/c++/4.8/bits/shared_ptr_base.h:807
#14 0x0873047a in std::shared_ptr<i2p::client::SAMSocket>::shared_ptr<i2p::client::SAMSocket> (this=0xadcf7d28, __r=std::weak_ptr (expired, weak 2) 0xb3f2cb00) at /usr/include/c++/4.8/bits/shared_ptr.h:249
#15 0x0872d8a6 in std::enable_shared_from_this<i2p::client::SAMSocket>::shared_from_this (this=0xb3f2cb00) at /usr/include/c++/4.8/bits/shared_ptr.h:557
#16 0x087276ed in i2p::client::SAMSocket::Terminate (this=0xb3f2cb00) at /opt/i2pd/SAM.cpp:42
#17 0x0872761b in i2p::client::SAMSocket::~SAMSocket (this=0xb3f2cb00, __in_chrg=<optimized out>) at /opt/i2pd/SAM.cpp:27
#18 0x0873eddb in __gnu_cxx::new_allocator<i2p::client::SAMSocket>::destroy<i2p::client::SAMSocket> (this=0xb3f2cafc, __p=0xb3f2cb00) at /usr/include/c++/4.8/ext/new_allocator.h:124
#19 0x0873ecd7 in std::allocator_traits<std::allocator<i2p::client::SAMSocket> >::_S_destroy<i2p::client::SAMSocket> (__a=..., __p=0xb3f2cb00) at /usr/include/c++/4.8/bits/alloc_traits.h:281
#20 0x0873ec08 in std::allocator_traits<std::allocator<i2p::client::SAMSocket> >::destroy<i2p::client::SAMSocket> (__a=..., __p=0xb3f2cb00) at /usr/include/c++/4.8/bits/alloc_traits.h:405
#21 0x0873ea9c in std::_Sp_counted_ptr_inplace<i2p::client::SAMSocket, std::allocator<i2p::client::SAMSocket>, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=0xb3f2caf0) at /usr/include/c++/4.8/bits/shared_ptr_base.h:407
#22 0x086870d0 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0xb3f2caf0) at /usr/include/c++/4.8/bits/shared_ptr_base.h:144
#23 0x086830b0 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=0xb3f02c5c, __in_chrg=<optimized out>) at /usr/include/c++/4.8/bits/shared_ptr_base.h:546
#24 0x086cbf02 in std::__shared_ptr<i2p::client::SAMSocket, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=0xb3f02c58, __in_chrg=<optimized out>) at /usr/include/c++/4.8/bits/shared_ptr_base.h:781
#25 0x086cbf4f in std::shared_ptr<i2p::client::SAMSocket>::~shared_ptr (this=0xb3f02c58, __in_chrg=<optimized out>) at /usr/include/c++/4.8/bits/shared_ptr.h:93
#26 0x0872d211 in std::_Head_base<0u, std::shared_ptr<i2p::client::SAMSocket>, false>::~_Head_base (this=0xb3f02c58, __in_chrg=<optimized out>) at /usr/include/c++/4.8/tuple:128
#27 0x0872d381 in std::_Tuple_impl<0u, std::shared_ptr<i2p::client::SAMSocket>, std::_Placeholder<1> >::~_Tuple_impl (this=0xb3f02c58, __in_chrg=<optimized out>) at /usr/include/c++/4.8/tuple:229
#28 0x0872d3a5 in std::tuple<std::shared_ptr<i2p::client::SAMSocket>, std::_Placeholder<1> >::~tuple (this=0xb3f02c58, __in_chrg=<optimized out>) at /usr/include/c++/4.8/tuple:521
#29 0x0872d3f2 in std::_Bind<std::_Mem_fn<void (i2p::client::SAMSocket::*)(std::shared_ptr<i2p::data::LeaseSet>)> (std::shared_ptr<i2p::client::SAMSocket>, std::_Placeholder<1>)>::~_Bind() (this=0xb3f02c50, __in_chrg=<optimized out>) at /usr/include/c++/4.8/functional:1280
#30 0x08734ebe in std::_Function_base::_Base_manager<std::_Bind<std::_Mem_fn<void (i2p::client::SAMSocket::*)(std::shared_ptr<i2p::data::LeaseSet>)> (std::shared_ptr<i2p::client::SAMSocket>, std::_Placeholder<1>)> >::_M_destroy(std::_Any_data&, std::integral_constant<bool, false>) (__victim=...) at /usr/include/c++/4.8/functional:1926
#31 0x08731e95 in std::_Function_base::_Base_manager<std::_Bind<std::_Mem_fn<void (i2p::client::SAMSocket::*)(std::shared_ptr<i2p::data::LeaseSet>)> (std::shared_ptr<i2p::client::SAMSocket>, std::_Placeholder<1>)> >::_M_manager(std::_Any_data&, std::_Any_data const&, std::_Manager_operation) (__dest=..., __source=..., __op=std::__destroy_functor) at /usr/include/c++/4.8/functional:1950
#32 0x0867ae99 in std::_Function_base::~_Function_base (this=0xadcf8098, __in_chrg=<optimized out>) at /usr/include/c++/4.8/functional:2030
#33 0x0868246b in std::function<void (std::shared_ptr<i2p::data::LeaseSet>)>::~function() (this=0xadcf8098, __in_chrg=<optimized out>) at /usr/include/c++/4.8/functional:2174
#34 0x087c8d39 in std::_Head_base<2u, std::function<void (std::shared_ptr<i2p::data::LeaseSet>)>, false>::~_Head_base() (this=0xadcf8098, __in_chrg=<optimized out>) at /usr/include/c++/4.8/tuple:128
#35 0x087c8d5d in std::_Tuple_impl<2u, std::function<void (std::shared_ptr<i2p::data::LeaseSet>)> >::~_Tuple_impl() (this=0xadcf8098, __in_chrg=<optimized out>) at /usr/include/c++/4.8/tuple:229
#36 0x087c8d81 in std::_Tuple_impl<1u, i2p::data::Tag<32>, std::function<void (std::shared_ptr<i2p::data::LeaseSet>)> >::~_Tuple_impl() (this=0xadcf8098, __in_chrg=<optimized out>) at /usr/include/c++/4.8/tuple:229
#37 0x087c8da5 in std::_Tuple_impl<0u, i2p::client::ClientDestination*, i2p::data::Tag<32>, std::function<void (std::shared_ptr<i2p::data::LeaseSet>)> >::~_Tuple_impl() (this=0xadcf8098, __in_chrg=<optimized out>) at /usr/include/c++/4.8/tuple:229
#38 0x087c8dc9 in std::tuple<i2p::client::ClientDestination*, i2p::data::Tag<32>, std::function<void (std::shared_ptr<i2p::data::LeaseSet>)> >::~tuple() (this=0xadcf8098, __in_chrg=<optimized out>) at /usr/include/c++/4.8/tuple:388
#39 0x087c8df0 in std::_Bind<std::_Mem_fn<void (i2p::client::ClientDestination::*)(i2p::data::Tag<32> const&, std::function<void (std::shared_ptr<i2p::data::LeaseSet>)>)> (i2p::client::ClientDestination*, i2p::data::Tag<32>, std::function<void (std::shared_ptr<i2p::data::LeaseSet>)>)>::~_Bind() (this=0xadcf8090, __in_chrg=<optimized out>) at /usr/include/c++/4.8/functional:1280
#40 0x087d02b5 in boost::asio::detail::completion_handler<std::_Bind<std::_Mem_fn<void (i2p::client::ClientDestination::*)(i2p::data::Tag<32> const&, std::function<void (std::shared_ptr<i2p::data::LeaseSet>)>)> (i2p::client::ClientDestination*, i2p::data::Tag<32>, std::function<void (std::shared_ptr<i2p::data::LeaseSet>)>)> >::do_complete(boost::asio::detail::task_io_service*, boost::asio::detail::task_io_service_operation*, boost::system::error_code const&, unsigned int) (owner=0xb3f0dc88, base=0xb3f0f468) at /usr/include/boost/asio/detail/completion_handler.hpp:59
#41 0x0867c7f7 in boost::asio::detail::task_io_service_operation::complete (this=0xb3f0f468, owner=..., ec=..., bytes_transferred=0) at /usr/include/boost/asio/detail/task_io_service_operation.hpp:37
#42 0x0867ee88 in boost::asio::detail::task_io_service::do_run_one (this=0xb3f0dc88, lock=..., this_thread=..., ec=...) at /usr/include/boost/asio/detail/impl/task_io_service.ipp:384
#43 0x0867ea15 in boost::asio::detail::task_io_service::run (this=0xb3f0dc88, ec=...) at /usr/include/boost/asio/detail/impl/task_io_service.ipp:153
#44 0x0867f194 in boost::asio::io_service::run (this=0xb3f08ab0) at /usr/include/boost/asio/impl/io_service.ipp:59
#45 0x087c49fd in i2p::client::ClientDestination::Run (this=0xb3f08a40) at /opt/i2pd/Destination.cpp:111
#46 0x087d4892 in std::_Mem_fn<void (i2p::client::ClientDestination::*)()>::operator()<, void>(i2p::client::ClientDestination*) const (this=0xb3f010dc, __object=0xb3f08a40) at /usr/include/c++/4.8/functional:601
#47 0x087d4845 in std::_Bind<std::_Mem_fn<void (i2p::client::ClientDestination::*)()> (i2p::client::ClientDestination*)>::__call<void, , 0u>(std::tuple<>&&, std::_Index_tuple<0u>) (this=0xb3f010dc, __args=<unknown type in /opt/i2pd/out/i2pd, CU 0xbb75ea, DIE 0xc52617>) at /usr/include/c++/4.8/functional:1296
#48 0x087d47b1 in std::_Bind<std::_Mem_fn<void (i2p::client::ClientDestination::*)()> (i2p::client::ClientDestination*)>::operator()<, void>() (this=0xb3f010dc) at /usr/include/c++/4.8/functional:1355
#49 0x087d46a5 in std::_Bind_simple<std::_Bind<std::_Mem_fn<void (i2p::client::ClientDestination::*)()> (i2p::client::ClientDestination*)> ()>::_M_invoke<>(std::_Index_tuple<>) (this=0xb3f010dc) at /usr/include/c++/4.8/functional:1732
#50 0x087d452f in std::_Bind_simple<std::_Bind<std::_Mem_fn<void (i2p::client::ClientDestination::*)()> (i2p::client::ClientDestination*)> ()>::operator()() (this=0xb3f010dc) at /usr/include/c++/4.8/functional:1720
#51 0x087d440c in std::thread::_Impl<std::_Bind_simple<std::_Bind<std::_Mem_fn<void (i2p::client::ClientDestination::*)()> (i2p::client::ClientDestination*)> ()> >::_M_run() (this=0xb3f010d0) at /usr/include/c++/4.8/thread:115
#52 0xb705580e in ?? () from /usr/lib/i386-linux-gnu/libstdc++.so.6
#53 0xb712ff70 in start_thread (arg=0xadcf8b40) at pthread_create.c:312
#54 0xb6ed850e in clone () at ../sysdeps/unix/sysv/linux/i386/clone.S:129
mlt commented 9 years ago

I just call CloseStream in destructor compared to Terminate in the former patch.

2015-Jun-24 14:26:58.757640,Thread=0x00005140,debug,SAMSocket,i2p::client::SAMSocket::SAMSocket @ 0x0909EFF8
2015-Jun-24 14:32:05.317843,Thread=0x00005700,debug,SAMSocket,i2p::client::SAMSocket::Terminate @ 0x0909EFF8
2015-Jun-24 14:32:05.323842,Thread=0x00005700,debug,SAMSocket,i2p::client::SAMSocket::CloseStream @ 0x0909EFF8
2015-Jun-24 14:32:05.360845,Thread=0x00005700,debug,SAMSocket,i2p::client::SAMSocket::Terminate @ 0x0909EFF8
2015-Jun-24 14:32:05.366845,Thread=0x00005700,debug,SAMSocket,i2p::client::SAMSocket::CloseStream @ 0x0909EFF8
... posted to SAM bridge thread
2015-Jun-24 14:32:05.431849,Thread=0x00005140,debug,SAMSocket,i2p::client::SAMSocket::TerminateSession @ 0x0909EFF8
2015-Jun-24 14:32:05.439850,Thread=0x00005140,debug,SAMSocket,i2p::client::SAMSocket::TerminateSession @ 0x0909EFF8
2015-Jun-24 14:32:05.456850,Thread=0x00005140,debug,SAMSocket,i2p::client::SAMSocket::~SAMSocket @ 0x0909EFF8
2015-Jun-24 14:32:05.473851,Thread=0x00005140,debug,SAMSocket,i2p::client::SAMSocket::CloseStream @ 0x0909EFF8
... memory being reused for another one
2015-Jun-24 14:32:08.120852,Thread=0x00005140,debug,SAMSocket,i2p::client::SAMSocket::SAMSocket @ 0x0909EFF8
2015-Jun-24 14:32:50.137697,Thread=0x00005140,debug,SAMSocket,i2p::client::SAMSocket::Terminate @ 0x0909EFF8
2015-Jun-24 14:32:50.146706,Thread=0x00005140,debug,SAMSocket,i2p::client::SAMSocket::CloseStream @ 0x0909EFF8
2015-Jun-24 14:32:50.156101,Thread=0x00005140,debug,SAMSocket,i2p::client::SAMSocket::TerminateSession @ 0x0909EFF8

There are still multiple calls to same functions, but as you said it is alright. So far it didn't die on win32.

orignal commented 9 years ago

Since ClientDestination is shared_ptr now, this issue should be fixed. If not, try and reopen it again