[patch][8799] Thread safe BGQueue updates

[Auntie Mangos]

Nice to see people working on finally getting mt stuff into mangos.

You might want to sync with Ambal: http://getmangos.eu/community/showthread.php?p=90710#post90710

[Triplex]

hello,

i created small change in bg queues code to prevent queue crashes with mtmaps patch.

It is not tested, but it is able to build and it should work.

version 0,6:

http://paste2.org/p/504012

I 'm writing thread code for the first time, i know bases about how mutex / locks should work.... but i know nothing about ACE...

Any suggestions are welcome.

[Triplex]

edited:

Thx to Zor for help with mutex in BattleGroundMgr scheduler.

Patch is ready to be tested.

to Leak: This patch is based on Ambal's patch ... thx to him too...

[nanounico]

edited:

Thx to Zor for help with mutex in BattleGroundMgr scheduler.

Patch is ready to be tested.

to Leak: This patch is based on Ambal's patch ... thx to him too...

Triplex, Can you update to latest rev please

[Triplex]

updated to the latest rev.

Pls test it, it would not be cool to apply patch without testing, as i do so often ....

[divisorxx]

I already have it on my server... tested with Rev 8579 and ~750 characters ON, finaly, not one crash due bgqueue update using derek's mtmaps. Thanks!

Only one due this assert, but it's not related at all, and it's and old crash, I posted here not to want something just to show you that it's not related.

       const uint32& GetUInt32Value( uint16 index ) const
       {
           ASSERT( index < m_valuesCount || PrintIndexError( index , false) );
           return m_uint32Values[ index ];
       }

#1  0x00007f057bd39960 in abort () from /lib64/libc.so.6
No symbol table info available.
#2  0x00007f057bd3137f in __assert_fail () from /lib64/libc.so.6
No symbol table info available.
#3  0x00000000004d5715 in Object::GetUInt32Value (this=<value optimized out>,
   index=<value optimized out>) at ../../../src/game/Object.h:166
       st = {static SYMBUFSIZ = 4096,
 buf_ = "./mangos-worldd(_ZNK5Guild12CanStoreItemEhhRSt6vectorI17GuildItemPosC
 static UNSUPPORTED = 0x7f057e3faaa0 "<stack traces unsupported platform>",
 static UNABLE_TO_GET_TRACE = 0x7f057e3faad0 "<unable to get trace>"}
       __FUNCTION__ = "GetUInt32Value"
       __PRETTY_FUNCTION__ = "const uint32& Object::GetUInt32Value(uint16) con
#4  0x00000000007694c8 in Guild::_CanStoreItem_InTab (this=0x435efa0,
   tab=<value optimized out>, dest=@0x451c7ef0, count=<value optimized out>,
   merge=<value optimized out>, pSrcItem=0x7f05192bfb70, skip_slot=255 'ÿ')
   at ../../../src/game/Object.h:142
       pItem2 = <value optimized out>
       j = <value optimized out>
#5  0x0000000000769a0d in Guild::CanStoreItem (this=0x435efa0,
   tab=<value optimized out>, slot=255 'ÿ', dest=@0x451c7ef0, count=1,
   pItem=0x7f05192bfb70, swap=false) at ../../../src/game/Guild.cpp:1954
       res = 0 '\\0'
       res = 0 '\\0'
#6  0x00000000007700bd in Guild::MoveFromCharToBank (this=0x435efa0,
   pl=0x7f05034db9c0, PlayerBag=<value optimized out>,
   PlayerSlot=<value optimized out>, BankTab=<value optimized out>,
   BankTabSlot=<value optimized out>, SplitedAmount=0)
   at ../../../src/game/Guild.cpp:2342
       dest = {<std::_Vector_base<GuildItemPosCount, std::allocator<GuildItemP
   _M_impl = {<std::allocator<GuildItemPosCount>> = {<__gnu_cxx::new_allocator
     _M_finish = 0x0, _M_end_of_storage = 0x0}}, <No data fields>}
       msg = <value optimized out>
       pItemBank = (class Item *) 0x0
       pItemChar = (class Item *) 0x7f05192bfb70
#7  0x00000000007745b6 in WorldSession::HandleGuildBankSwapItems (
   this=0x2c3474b0, recv_data=@0x7f0556ab9650)
   at ../../../src/game/GuildHandler.cpp:1143
       BankTab = 0 '\\0'
       BankTabDst = 102 'f'
       unk2 = 0 '\\0'
       SplitedAmount = 0
       pl = (class Player *) 0x7f05034db9c0
       GoGuid = <value optimized out>
       AutoStore = 0 '\\0'
       ToChar = 0 '\\0'
       AutoStoreCount = 0
       AutoStoreCount = 0
       GuildId = <value optimized out>
       BankTabSlot = 255 'ÿ'
       PlayerBag = 20 '\\024'
       BankTabSlotDst = 240 'ð'
       ItemEntry = 0
       BankToBank = 0 '\\0'
       PlayerSlot = 5 '\\005'
       unk1 = 4294967295
       pGuild = (Guild *) 0x435efa0
#8  0x0000000000727902 in WorldSession::Update (this=0x2c3474b0)
   at ../../../src/game/../shared/Log.h:119
       packet = (WorldPacket *) 0x7f0556ab9650
       currTime = <value optimized out>

[przemratajczak]

compiling on VC90 i get warning

2>..\\..\\src\\game\\BattleGroundMgr.cpp(2041) : warning C4293: '<<' : shift count negative or too big, undefined behavior

[Lynx3d]

I think this is a bad idea (on a 32bit platform at least):

(arenaRating << 32)

It will not automatically be promoted to 64bit, and the behaviour of a shift that is larger or equal to the promoted width of the left operand is undefined.

Research suggest that usually "<< 32" is equal to doing nothing, but it really can result in anything on 32bit values.

Cast it to uint64 before shifting (and hope no one uses VS2005 without SP1, accidently found that while googling...)

[Triplex]

added uint64 cast to prevent warning and possibly unexpected behaviour.

that crash is about guild_bank, you can report bug.

Edit:

I tested patch, and fixed problems with leaving queues - i forgot "!" in one condition ....

So patch is almost ready to be commited.

New version added

[divisorxx]

Sorry When I said that I tested this patch i mean Ambal's Patch... i think it was the same... i will tested this one now.

[nanounico]

Error at Compile:

In file included from /Sources/src/game/BattleGround.cpp:22:

Sources/src/game/BattleGroundMgr.h:96: error: ‘ACE_Recursive_Thread_Mutex’ does not name a type

[Triplex]

v0,6 - fixed compilation error. Thx for report and to vladimir and balrok for discussion .

[nanounico]

Can you update for latest rev please.

[Triplex]

commited in [8799]. i tested it myself :mad:.

[nanounico]

Congrats for port in master

my before post was made some few minuts before 8799, sorry.

[The_Game_Master]

I didn't quite understand for what this patch is for, exept to be used with mtmaps for a stable core. Is that right?

[Triplex]

yes it fixes bg queue crashes when map updates are executed in threads. For single threaded core updates this patch doesn't change anything.

[leak]

Any chance of this getting backported?