TFS 0.X Crash: ChatChannel removeUser

[GSMaster]

hello, sometimes a server crash occurs (or someone else causes it):

gdb log:

 -  Thread 2 "tfs_2" received signal SIGSEGV, Segmentation fault.
 -  [Switching to Thread 0x7ffff4c6c700 (LWP 32327)]
 -  std::_Rb_tree<unsigned int, std::pair<unsigned int const, Player*>, std::_Select1st<std::pair<unsigned int const, Player*> >, std::less<unsigned int>, std::allocator<std::pair<unsigned int const, Player*> > >::find (this=this@entry=0x70706060707076f, __k=@0x7ffff4c6b64c: 268469330) at /usr/include/c++/8/bits/stl_tree.h:2539
 -  2539          iterator __j = _M_lower_bound(_M_begin(), _M_end(), __k);



 -  Thread 2 (Thread 0x7ffff4c6c700 (LWP 32327)):
 -  #0  std::_Rb_tree<unsigned int, std::pair<unsigned int const, Player*>, std::_Select1st<std::pair<unsigned int const, Player*> >, std::less<unsigned int>, std::allocator<std::pair<unsigned int const, Player*> > >::find (this=this@entry=0x70706060707076f, __k=@0x7ffff4c6b64c: 268469330) at /usr/include/c++/8/bits/stl_tree.h:2539
 -          __j = <optimized out>
 -  #1  0x000055555559e503 in std::map<unsigned int, Player*, std::less<unsigned int>, std::allocator<std::pair<unsigned int const, Player*> > >::find (__x=@0x7ffff4c6b64c: 268469330, this=0x70706060707076f) at /usr/include/c++/8/bits/stl_map.h:1168
 -  No locals.
 -  #2  ChatChannel::removeUser (this=0x707060607070707, player=player@entry=0x7fffe8a75350) at chat.cpp:154
 -          it = <optimized out>
 -  #3  0x000055555559e9fd in Chat::removeUserFromAllChannels (this=0x55555583c080 <g_chat>, player=player@entry=0x7fffe8a75350) at chat.cpp:532
 -          it = {first = 1799, second = 0x707060607070707}
 -  #4  0x000055555570c93d in Player::onCreatureDisappear (this=0x7fffe8a75350, creature=<optimized out>, isLogout=<optimized out>) at player.cpp:2146
 -          saved = <optimized out>
 -  #5  0x00005555555fcf3b in Game::removeCreature (this=0x55555583c1c0 <g_game>, creature=0x7fffe8a75350, isLogout=isLogout@entry=true) at game.cpp:1066
 -          tile = 0x7fffbe55be90
 -          list = std::__cxx11::list = {[0] = 0x7fffe8a75350, [1] = 0x7fffc05f7ee0, [2] = 0x7fffccb939e0, [3] = 0x7fffccae1150}
 -          it = 0x7fffe8a75350
 -          player = <optimized out>
 -          oldStackPosVector = std::vector of length 23456248162376, capacity 23456248161672 = {<error reading variable oldStackPosVector (Cannot access memory at address 0x0)>
 -          i = <optimized out>
 -          oldIndex = <optimized out>
 -  #6  0x0000555555728374 in ProtocolGame::logout (this=0x555555928c80, displayEffect=<optimized out>, forced=<optimized out>, executeLogout=<optimized out>) at protocolgame.cpp:427
 -  No locals.
 -  #7  0x0000555555735ee9 in boost::_mfi::mf3<bool, ProtocolGame, bool, bool, bool>::operator() (a3=<optimized out>, a2=<optimized out>, a1=<optimized out>, p=<optimized out>, this=<optimized out>) at /usr/include/boost/bind/mem_fn_template.hpp:391
 -  No locals.
 -  #8  boost::_bi::list4<boost::_bi::value<ProtocolGame*>, boost::_bi::value<bool>, boost::_bi::value<bool>, boost::_bi::value<bool> >::operator()<bool, boost::_mfi::mf3<bool, ProtocolGame, bool, bool, bool>, boost::_bi::list0> (a=<synthetic pointer>..., f=..., this=<optimized out>) at /usr/include/boost/bind/bind.hpp:453
 -  No locals.
 -  #9  boost::_bi::bind_t<bool, boost::_mfi::mf3<bool, ProtocolGame, bool, bool, bool>, boost::_bi::list4<boost::_bi::value<ProtocolGame*>, boost::_bi::value<bool>, boost::_bi::value<bool>, boost::_bi::value<bool> > >::operator() (this=<optimized out>) at /usr/include/boost/bind/bind.hpp:1294
 -          a = <optimized out>
 -          a = <optimized out>
 -  #10 boost::detail::function::void_function_obj_invoker0<boost::_bi::bind_t<bool, boost::_mfi::mf3<bool, ProtocolGame, bool, bool, bool>, boost::_bi::list4<boost::_bi::value<ProtocolGame*>, boost::_bi::value<bool>, boost::_bi::value<bool>, boost::_bi::value<bool> > >, void>::invoke (function_obj_ptr=...) at /usr/include/boost/function/function_template.hpp:159
 -          f = <optimized out>
 -  #11 0x00005555555c2842 in boost::function0<void>::operator() (this=<optimized out>) at /usr/include/boost/function/function_template.hpp:682
 -  No locals.
 -  #12 std::_Function_handler<void (), boost::function<void ()> >::_M_invoke(std::_Any_data const&) (__functor=...) at /usr/include/c++/8/bits/std_function.h:297
 -  No locals.
 -  #13 0x00005555555bda7f in std::function<void ()>::operator()() const (this=<optimized out>) at /usr/include/c++/8/bits/std_function.h:682
 -  No locals.
 -  #14 boost::detail::function::void_function_obj_invoker0<std::function<void ()>, void>::invoke(boost::detail::function::function_buffer&) (function_obj_ptr=...) at /usr/include/boost/function/function_template.hpp:159
 -          f = <optimized out>
 -  #15 0x00005555555eefb4 in boost::function0<void>::operator() (this=0x555555b3f248) at /usr/include/boost/function/function_template.hpp:682
 -  No locals.
 -  #16 Task::operator() (this=0x555555b3f240) at dispatcher.h:41
 -  No locals.
 -  #17 Dispatcher::dispatcherThread (this=0x55555583d160 <g_dispatcher>) at dispatcher.cpp:69
 -          task = 0x555555b3f240
 -          outputPool = 0x5555558353e0 <OutputMessagePool::getInstance()::instance>
 -          taskLockUnique = {m = 0x55555583d170 <g_dispatcher+16>, is_locked = false}
 -  #18 0x00005555555f00d9 in boost::_mfi::mf0<void, Dispatcher>::operator() (p=<optimized out>, this=<optimized out>) at /usr/include/boost/bind/mem_fn_template.hpp:47
 -  No locals.
 -  #19 boost::_bi::list1<boost::_bi::value<Dispatcher*> >::operator()<boost::_mfi::mf0<void, Dispatcher>, boost::_bi::list0> (a=<synthetic pointer>..., f=..., this=<optimized out>) at /usr/include/boost/bind/bind.hpp:259
 -  No locals.
 -  #20 boost::_bi::bind_t<void, boost::_mfi::mf0<void, Dispatcher>, boost::_bi::list1<boost::_bi::value<Dispatcher*> > >::operator() (this=<optimized out>) at /usr/include/boost/bind/bind.hpp:1294
 -          a = <optimized out>
 -          a = <optimized out>
 -  #21 boost::detail::thread_data<boost::_bi::bind_t<void, boost::_mfi::mf0<void, Dispatcher>, boost::_bi::list1<boost::_bi::value<Dispatcher*> > > >::run (this=<optimized out>) at /usr/include/boost/thread/detail/thread.hpp:117
 -  No locals.
 -  #22 0x00007ffff7ba8615 in ?? () from /lib/x86_64-linux-gnu/libboost_thread.so.1.67.0
 -  No symbol table info available.
 -  #23 0x00007ffff7fa7fa3 in start_thread (arg=<optimized out>) at pthread_create.c:486
 -          ret = <optimized out>
 -          pd = <optimized out>
 -          now = <optimized out>
 -          unwind_buf = {cancel_jmp_buf = {{jmp_buf = {140737300055808, -4018973014395969550, 140737488345918, 140737488345919, 140737300055808, 93824995576960, 4018993018165796850, 4018990576528400370}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}}
 -          not_first_call = <optimized out>
 -  #24 0x00007ffff757b4cf in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
 -  No locals.
 -

player.cpp:2146

    g_chat.removeUserFromAllChannels(this);

chat.cpp:532

  for(PrivateChannelMap::iterator it = m_privateChannels.begin(); it != m_privateChannels.end(); ++it)
    {
line 532:        it->second->removeUser(player);
        if(player && it->second->getOwner() == player->getGUID())
            deleteChannel(player, it->second->getId());
    }

chat.cpp:154

bool ChatChannel::removeUser(Player* player)
{
    if(!player || player->isRemoved())
        return false;

line 154:    UsersMap::iterator it = m_users.find(player->getID());
    if(it == m_users.end())
        return false;

    m_users.erase(it);

 

[Gesior.pl]

@GSMaster
Can you post your protocolgame.cpp file? Do you know, if there was some message in server console when it crashed? Something about kicking player for 'wrong packet' or 'limit of packets'?

 

[ThaLeeeS]

Which operating system version are you running?

 

[Cziks]

Hi @Gesior.pl
I encountered exact the same problem in OTX2 distro. This occurred after 30 seconds~ when I made an xlog on one of the logged accounts.
I have only screenshot because I did not captured it as plain text.
Operating system: Ubuntu 18.04

player.cpp:1588

g_chat.removeUserFromChannels(this);

chat.cpp:505-525

void Chat::removeUserFromChannels(Player* player)
{
    if(!player || player->isRemoved())
        return;

    for(NormalChannelMap::iterator it = m_normalChannels.begin(); it != m_normalChannels.end(); ++it)
        it->second->removeUser(player);

    for(PartyChannelMap::iterator it = m_partyChannels.begin(); it != m_partyChannels.end(); ++it)
        it->second->removeUser(player);

    for(GuildChannelMap::iterator it = m_guildChannels.begin(); it != m_guildChannels.end(); ++it)
        it->second->removeUser(player);

    for(PrivateChannelMap::iterator it = m_privateChannels.begin(); it != m_privateChannels.end(); ++it)
    {
        it->second->removeUser(player);
        if(it->second->getOwner() == player->getGUID())
            deleteChannel(player, it->second->getId());
    }
}

removeUser chat.cpp:136-151

bool ChatChannel::removeUser(Player* player)
{
    if(!player)
        return false;

    UsersMap::iterator it = m_users.find(player->getID());
    if(it == m_users.end())
        return true;

    m_users.erase(it);
    CreatureEventList leaveEvents = player->getCreatureEvents(CREATURE_EVENT_CHANNEL_LEAVE);
    for(CreatureEventList::iterator it = leaveEvents.begin(); it != leaveEvents.end(); ++it)
        (*it)->executeChannel(player, m_id, m_users);

    return true;
}

protocolgame.cpp:

////////////////////////////////////////////////////////////////////////// Ope - Pastebin.com

Pastebin.com is the number one paste tool since 2002. Pastebin is a website where you can store text online for a set period of time.

pastebin.com

chat.cpp:141 points to: UsersMap::iterator it = m_users.find(player->getID());

 

[fabian766]

    for(PrivateChannelMap::iterator it = m_privateChannels.begin(); it != m_privateChannels.end(); ++it)
    {
        it->second->removeUser(player);
        if(it->second->getOwner() == player->getGUID())
            deleteChannel(player, it->second->getId());
    }

Iterator validity:
Iterators, pointers and references referring to elements removed by the function are invalidated.
All other iterators, pointers and references keep their validity.

TFS(even 1.4) have much more such bullshittery in code that violate STL/3rd party libraries documentation which result sooner or later in crashes.

 

[Cziks]

I figured out how to reproduce it but I will not give it here because some people might use it to crash servers.

I have read about iterator invalidation in STL map container mentioned by @fabian766 in previous post. After calling erase function which is used in deleteChannel() I updated the iterator 'it'. This prevented crashes for me. Please let me know if I can improve it in some way.
@GSMaster if you are still looking for a solution you can test mine.

Before:

    for(PrivateChannelMap::iterator it = m_privateChannels.begin(); it != m_privateChannels.end(); ++it)
    {
        it->second->removeUser(player);
        if(it->second->getOwner() == player->getGUID())
            deleteChannel(player, it->second->getId());
    }

After:

    PrivateChannelMap::iterator it = m_privateChannels.begin();
    while(it != m_privateChannels.end())
    {
        it->second->removeUser(player);
        if(it->second->getOwner() == player->getGUID())
        {
            it->second->closeChannel();
            it = m_privateChannels.erase(it);
        }
        else
        {
            ++it;
        }
    }

 

[Gesior.pl]

I figured out how to reproduce it but I will not give it here because some people might use it to crash servers.

Testing it on some popular 0.4 server. They use GitHub - Fir3element/3777 (https://github.com/Fir3element/3777)

 

[Serene]

If you have problems compiling Cziks solution maybe you should try this alternative solution:

 // first loop - remove player from all private channels
    PrivateChannelMap::iterator it = m_privateChannels.begin();
    while(it != m_privateChannels.end())
    {
        it->second->removeUser(player);
        ++it;
    }

    // second loop - delete channel of which player is owner - expecting that there should be one
    it = m_privateChannels.begin();
    while(it != m_privateChannels.end())
    {
        if(it->second->getOwner() == player->getGUID())
        {
            it->second->closeChannel();
            // after closing channel stop loop
            break;
        }
        ++it;
    }

Credits to Gesior.pl

 

[potinho]

If you have problems compiling Cziks solution maybe you should try this alternative solution:

 // first loop - remove player from all private channels
    PrivateChannelMap::iterator it = m_privateChannels.begin();
    while(it != m_privateChannels.end())
    {
        it->second->removeUser(player);
        ++it;
    }

    // second loop - delete channel of which player is owner - expecting that there should be one
    it = m_privateChannels.begin();
    while(it != m_privateChannels.end())
    {
        if(it->second->getOwner() == player->getGUID())
        {
            it->second->closeChannel();
            // after closing channel stop loop
            break;
        }
        ++it;
    }

Credits to Gesior.pl

works here, thank you

 

[rooyo]

i need help for fix, i pay PW
0.3.6pl

 

[rooyo]

Bump im good pay priv...