Jump to content
TrinityCore

jackpoz

Developers
  • Posts

    172
  • Joined

  • Last visited

  • Days Won

    15

Posts posted by jackpoz

  1. Just tested sending CMSG_BATTLEFIELD_MGR_ENTRY_INVITE_RESPONSE with botfarm with 4 bots at Dalaran, Honor Hold, Darnassus and Elwynn Forest with 3 Map Threads and helgrind running and all in the same raid with me too, I saw them leaving the party then worldserver stopped with "Killed" message in the console.
    We can mark this issue as "reproduced".

    This is the full race condition log, you can see 2 MapThreads both calling indeed the function you said

    Possible data race during read of size 8 at 0x53CEBA90 by thread #8
    Locks held: none
       at 0x163B512: LinkedListHead::isEmpty() const (LinkedList.h:102)
       by 0x163B53B: LinkedListHead::getFirst() (in /home/jackpoz/trinity/bin/worldserver)
       by 0x17AA6ED: RefManager<Group, Player>::getFirst() (RefManager.h:33)
       by 0x17A9089: GroupRefManager::getFirst() (GroupRefManager.h:31)
       by 0x1A85AD6: Group::DelinkMember(ObjectGuid) (Group.cpp:2395)
       by 0x1A7D871: Group::RemoveMember(ObjectGuid, RemoveMethod const&, ObjectGuid, char const*) (Group.cpp:538)
       by 0x1E6B514: Battlefield::AddOrSetPlayerToCorrectBfGroup(Player*) (Battlefield.cpp:509)
       by 0x1E6A978: Battlefield::PlayerAcceptInviteToWar(Player*) (Battlefield.cpp:382)
       by 0x1F57F06: WorldSession::HandleBfEntryInviteResponse(WorldPacket&) (BattlefieldHandler.cpp:158)
       by 0x1C60984: WorldSession::Update(unsigned int, PacketFilter&) (WorldSession.cpp:321)
       by 0x1B865B9: Map::Update(unsigned int) (Map.cpp:656)
       by 0x1F9323F: MapUpdateRequest::call() (MapUpdater.cpp:42)

    This conflicts with a previous write of size 8 by thread #7
    Locks held: none
       at 0x173504C: LinkedListElement::delink() (LinkedList.h:57)
       by 0x18AD488: Reference<Group, Player>::unlink() (Reference.h:64)
       by 0x18923DB: Player::SetGroup(Group*, signed char) (Player.cpp:22467)
       by 0x1A7D688: Group::RemoveMember(ObjectGuid, RemoveMethod const&, ObjectGuid, char const*) (Group.cpp:509)
       by 0x1E6B514: Battlefield::AddOrSetPlayerToCorrectBfGroup(Player*) (Battlefield.cpp:509)
       by 0x1E6A978: Battlefield::PlayerAcceptInviteToWar(Player*) (Battlefield.cpp:382)
       by 0x1F57F06: WorldSession::HandleBfEntryInviteResponse(WorldPacket&) (BattlefieldHandler.cpp:158)
       by 0x1C60984: WorldSession::Update(unsigned int, PacketFilter&) (WorldSession.cpp:321)
     Address 0x53ceba90 is 32 bytes inside a block of size 544 alloc'd
       at 0x4C2D577: operator new(unsigned long) (vg_replace_malloc.c:333)
       by 0x1AE50BF: WorldSession::HandleGroupInviteOpcode(WorldPacket&) (GroupHandler.cpp:164)
       by 0x1C60984: WorldSession::Update(unsigned int, PacketFilter&) (WorldSession.cpp:321)
       by 0x1D9C65A: World::UpdateSessions(unsigned int) (World.cpp:2712)
       by 0x1D9971C: World::Update(unsigned int) (World.cpp:2075)
       by 0x157B002: WorldUpdateLoop() (Main.cpp:380)
       by 0x1578C9B: main (Main.cpp:242)
     Block was alloc'd by thread #1

    The whole BattlefieldHandler.cpp looks unsafe, not sanitizing any input (all I had to do was send CMSG_BATTLEFIELD_MGR_ENTRY_INVITE_RESPONSE with Wintergrasp battlefield ID). For example WorldSession::HandleBfExitRequest() modifies Battlefield data even if the player is on another map.

    Battlefield-related packets that are supposed to be sent by the client only when already in the battle should check for mapid, the other ones should be set as PROCESS_THREADUNSAFE

    • Upvote 1
  2. Try log in with 2 characters
    do .bf enable 1 to enable wintergrasp, go to the area with .tele wintergrasp and join the que.
    Then use .bf start 1 to start it. Everyone should have got the joining message which when you click you get entered to new group.

    ​I tele'd to the area with 1 party member, joined the queue, .bf start 1, accepted: the other party members not in Wintergrasp didn't get any popup.

  3. Ah that's why I didn't get any report, I tested Random Battleground, not Wintergrasp.

    Wintergrasp is disabled by default and should be enabled only by developers who want to work on it, the config file describes it as "experimental" https://github.com/TrinityCore/TrinityCore/blob/3.3.5/src/server/worldserver/worldserver.conf.dist#L2268 .

    Your findings are most likely right and there might be many more issues related to Wintergrasp. I'll give it another try with Helgrind.

  4. https://www.javacodegeeks.com/2015/07/a-few-valid-reasons-to-reject-a-bug-fix.html

    Quote

    Bug fixes are not features; they must be small and focused. It’s a very typical mistake for programmers to get carried away while fixing a bug and introduce some refactoring together with a fix. The result is that the patch gets rather big and difficult to understand. I’m not against refactoring; it’s a very important and positive thing for a project, but do it separately after the bug is fixed and merged.

    No refactoring while fixing a bug!

     

    Quote

    Always fix one issue at a time — simple as that. No exceptions.

    Combining several bug fixes into a single pull request is a very bad practice. No matter how simple the fix is, keep it separate from others.

    • Upvote 1
  5. That's a known issue reported by travis too at https://travis-ci.org/TrinityCore/TrinityCore#L408 .

    You can check the build status on the https://github.com/TrinityCore/TrinityCore homepage, 6.x build status is "failing" at the moment; in these cases wait for someone to fix the build, hopefully this will save you all those steps next time.

×
×
  • Create New...