• There is NO official Otland's Discord server and NO official Otland's server list. The Otland's Staff does not manage any Discord server or server list. Moderators or administrator of any Discord server or server lists have NO connection to the Otland's Staff. Do not get scammed!

[Bug Report] Item destructor

Theofar

Member
Joined
Oct 18, 2011
Messages
62
Reaction score
11
Hello, recently I got a crash on my TFS 1.2 related to item destructor.

The referenced line is:

item->setParent(nullptr);

My teorie is that item is being removed from memory but without updating inventory vector from players.cpp.

So, player destructor will try to use a pointer for a item on inventory vector, and crashes server (read memory access violation).

If confirmed, it's present on every TFS. Maybe an old unknow bug or a very rare bug.

C++:
Thread 2 (Thread 0x7ffff5a51700 (LWP 31311)):
#0  0x00007fffce3ca680 in ?? ()
No symbol table info available.
#1  0x000000000059419e in Player::~Player (this=0x7fffcddc69e0) at /home/otserv1064/tfs/src/player.cpp:60
        item = 0x7fffce3ca690
        __for_range = @0x7fffcddc6fe8: {0x0, 0x7fffce6a7070, 0x0, 0x7fffcfa39c70, 0x0, 0x7fffce916dc0, 0x7fffce3ca690, 0x0, 0x0, 0x0, 0x7fffccfd5880}
        __for_begin = 0x7fffcddc7018
        __for_end = 0x7fffcddc7040
#2  0x00000000005943a2 in Player::~Player (this=0x7fffcddc69e0) at /home/otserv1064/tfs/src/player.cpp:71
No locals.
#3  0x000000000065009a in Creature::decrementReferenceCounter (this=0x7fffcddc69e0) at /home/otserv1064/tfs/src/creature.h:449
No locals.
#4  0x0000000000604f41 in Game::cleanup (this=0xa40ce0 <g_game>) at /home/otserv1064/tfs/src/game.cpp:3849
        creature = 0x7fffcddc69e0
        __for_range = Python Exception <class 'RuntimeError'> Type is not a template.:
@0xa411a8: Python Exception <class 'RuntimeError'> Type is not a template.:
{<No data fields>}
        __for_begin =
        __for_end =
#5  0x000000000060a212 in Game::checkCreatures (this=0xa40ce0 <g_game>, index=1) at /home/otserv1064/tfs/src/game.cpp:3199
        __func__ = "checkCreatures"
        checkCreatureList = @0xa410c8: {<No data fields>}
        it = Python Exception <class 'ValueError'> Cannot find type std::_List_iterator::_Node:

        end = Python Exception <class 'ValueError'> Cannot find type std::_List_iterator::_Node:

#6  0x000000000060b2ba in operator() (this=0x7fffce6b92d0, __object=0xa40ce0 <g_game>, __args#0=@0x7fffce6b92e0: 1) at /usr/include/c++/5/functional:600
No locals.
#7  0x000000000060f9bf in __call (this=0x7fffce6b92d0, __args=<unknown type in /home/otserv1064/tfs/tfs, CU 0x21b4b4, DIE 0x234c81>) at /usr/include/c++/5/functional:1074
No locals.
#8  0x0000000000615784 in std::_Bind<std::_Mem_fn<void (Game::*)(unsigned long)> (Game*, unsigned long)>::operator()<, void>() [clone .lto_priv.1926] (this=0x7fffce6b92d0) at /usr/include/c++/5/functional:1133
No locals.
#9  0x00000000005f6839 in std::_Function_handler<void (), std::_Bind<std::_Mem_fn<void (Game::*)(unsigned long)> (Game*, unsigned long)> >::_M_invoke(std::_Any_data const&) (__functor=...) at /usr/include/c++/5/functional:1871
No locals.
#10 0x0000000000570ffc in std::function::operator() (this=0x7fffceb4c968) at /usr/include/c++/5/functional:2267
No locals.
#11 0x000000000057101a in Task::operator() (this=0x7fffceb4c910) at /home/otserv1064/tfs/src/tasks.h:41
No locals.
#12 0x000000000058219b in Dispatcher::threadMain (this=0xa377c0 <g_dispatcher>) at /home/otserv1064/tfs/src/tasks.cpp:71
        task = 0x7fffceb4c910
        taskLockUnique = {_M_device = 0xa377d8 <g_dispatcher+24>, _M_owns = false}
        time_point = {__d = {__r = 1571575320674003775}}
#13 0x000000000058538f in std::_Mem_fn_base::operator() (this=0xabc6d0, __object=0xa377c0 <g_dispatcher>) at /usr/include/c++/5/functional:600
No locals.
#14 0x00000000005896e5 in std::_Bind_simple::_M_invoke (this=0xabc6c8) at /usr/include/c++/5/functional:1531
No locals.
#15 0x0000000000589709 in std::_Bind_simple::operator() (this=0xabc6c8) at /usr/include/c++/5/functional:1520
No locals.
#16 0x000000000058972c in std::thread::_Impl::_M_run (this=0xabc6b0) at /usr/include/c++/5/thread:115
No locals.
#17 0x00007ffff6449c80 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
No symbol table info available.
#18 0x00007ffff671a6ba in start_thread (arg=0x7ffff5a51700) at pthread_create.c:333
        __res = <optimized out>
        pd = 0x7ffff5a51700
        now = <optimized out>
        unwind_buf = {cancel_jmp_buf = {{jmp_buf = {140737314625280, -8600867084247919482, 0, 140737488347775, 140737314625984, 0, 8600889845969949830, 8600881430831535238}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}}
        not_first_call = <optimized out>
        pagesize_m1 = <optimized out>
        sp = <optimized out>
        freesize = <optimized out>
        __PRETTY_FUNCTION__ = "start_thread"
#19 0x00007ffff7b1441d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
No locals.
 
Have you found any steps to reproduce this?
 
If confirmed, it's present on every TFS. Maybe an old unknow bug or a very rare bug.
Did you actually test every single TFS and confirm it's a core issue? Could you specify what exactly you tested and how?
 
We got a hint, that occurs when using small stones.

Removing the feature that small stones fall in the ground, seems to "fix".
 
There is one lua setup that crash server because of freed item pointer. It was really tricky to track this out since the crash not happen immediately but after quite long time when item object is accessed.

Lua:
local someItem = Item(doCreateItemEx(ITEM_GOLD_COIN, 100))

-- do something different that triggers other lua event like onEquip - it calls ScriptEnvironment::resetEnv()
player:addItem(2661) -- scarf, must have entry in movements.xml with script not function
player:addExperience(100) -- triggers Player.onGainExperience event

player:addItemEx(someItem)

-- crash will happen after some time when someItem is removed by Game object in ToReleaseItems and accessed again anywhere in the program
 
Last edited:
There is one lua setup that crash server because of freed item pointer. It was really tricky to track this out since the crash not happen immediately but after quite long time when item object is accessed.

Lua:
local someItem = doCreateItemEx(ITEM_GOLD_COIN, 100)

-- do something different that triggers other lua event like onEquip - it calls ScriptEnvironment::resetEnv()
player:addItem(2661) -- scarf, must have entry in movements.xml with script not function
player:addExperience(100) -- triggers Player.onGainExperience event

player:addItemEx(someItem)

-- crash will happen after some time when someItem is removed by Game object in ToReleaseItems and accessed again anywhere in the program


~Just hand over the object after you have created it line 7 move it to line 2 for example ;)~
~the uniqueID system of the articles is temporary and they are used in a deterministic way~


Since doCreateItemEx returns a uniqueID you have to search for the item first and then add to player
Example: player:addItemEx(Item(someItem))

The crashes is produced because TFS tries to create a pointer of the userdata on the lua-stack, but if in your case it is not a userdata but a number that is the uniqueID

We still have to have certain rules for scripting in TFS, it is possible that future lua will not crashes the interpreter ;)
 
Last edited:
There is one lua setup that crash server because of freed item pointer. It was really tricky to track this out since the crash not happen immediately but after quite long time when item object is accessed.

Lua:
local someItem = doCreateItemEx(ITEM_GOLD_COIN, 100)

-- do something different that triggers other lua event like onEquip - it calls ScriptEnvironment::resetEnv()
player:addItem(2661) -- scarf, must have entry in movements.xml with script not function
player:addExperience(100) -- triggers Player.onGainExperience event

player:addItemEx(someItem)

-- crash will happen after some time when someItem is removed by Game object in ToReleaseItems and accessed again anywhere in the program

no success to reproduce here, but it makes sense
following the log, you see that its while trying to remove creature (decrementReferenceCounter), so i think it is related to logout somewhere
 
Last edited:
~Just hand over the object after you have created it line 7 move it to line 2 for example ;)~
~the uniqueID system of the articles is temporary and they are used in a deterministic way~


Since doCreateItemEx returns a uniqueID you have to search for the item first and then add to player
Example: player:addItemEx(Item(someItem))

The crashes is produced because TFS tries to create a pointer of the userdata on the lua-stack, but if in your case it is not a userdata but a number that is the uniqueID

We still have to have certain rules for scripting in TFS, it is possible that future lua will not crashes the interpreter ;)
This is not thing. The problem is that engine really removes someItem from memory even if you still refer to it in lua script. It's because ScriptEnvironment::resetEnv() is being called and item has still VirtualCylinder as parent. It does not check whether you have reference to it in lua script or not.
I know the difference. I've edited the example now so it fits what I mentioned. Just forgot doCreateItemEx does not return Userdata. The example now is correct.

Lua:
-- create any item and get Userdata reference
local someItem = Item(doCreateItemEx(ITEM_GOLD_COIN, 100))
-- do something different that triggers other lua event like onEquip - it calls ScriptEnvironment::resetEnv()
player:addItem(2661) -- scarf, must have entry in movements.xml with script not function
player:addExperience(100) -- triggers Player.onGainExperience event

player:addItemEx(someItem)
-- crash will happen after some time when someItem is removed by Game object in ToReleaseItems and accessed again anywhere in the program
 
Last edited:
This is not thing. The problem is that engine really removes someItem from memory even if you still refer to it in lua script. It's because ScriptEnvironment::resetEnv() is being called and item has still VirtualCylinder as parent. It does not check whether you have reference to it in lua script or not.
I know the difference. I've edited the example now so it fits what I mentioned. Just forgot doCreateItemEx does not return Userdata. The example now is correct.

Lua:
-- create any item and get Userdata reference
local someItem = Item(doCreateItemEx(ITEM_GOLD_COIN, 100))
-- do something different that triggers other lua event like onEquip - it calls ScriptEnvironment::resetEnv()
player:addItem(2661) -- scarf, must have entry in movements.xml with script not function
player:addExperience(100) -- triggers Player.onGainExperience event

player:addItemEx(someItem)
-- crash will happen after some time when someItem is removed by Game object in ToReleaseItems and accessed again anywhere in the program

understood but it still not crashing here with this example
any idea for hotfix on it?
thanks for helping
 
Changing item position makes the item change. Sometimes the item is still accessible, sometimes it isn't, sometimes it crash the server. My advice is to do everything you had to do with the item before adding it to player inventory. Also don't use userdata in addevent.
 
xDDD

why all you guys didnt check (pardon if you check, dear reader) if thing you want to read/write exists xD
most of tfs unknown crashes in my past were like this
 
xDDD

why all you guys didnt check (pardon if you check, dear reader) if thing you want to read/write exists xD
most of tfs unknown crashes in my past were like this
My love,

if you do
if (ptr) ptr->something()

this can still throw in some cases, e.g. multi-threaded environment. Checking is not as trivial as it sounds.

Analyzing the call-stack, in the reported bug, a check is done one line before the code has a memory violation.

Don't be writing arrogant comments like this.
 
Of course it can, that's why people invented thread safety, mutexes etc. It's still simple
 
Of course it can, that's why people invented thread safety, mutexes etc. It's still simple
To use mutual exclusive sections, many parts of the code have to be changed. And mutual exclusive sections cannot be simply used without introducing possible side-effects. No code can ever be simple.
 
To use mutual exclusive sections
xD
It was enough to write thread safe code right from the start.

b9c25cda2d5ffa96bd2b9f4c2361cbc9.jpg
 
Back
Top