Fix a database race condition in authserver that would delay the account ban expiry by 1 login because the query that would have removed the ban was executed asynchronously.
Fix memory leak in Auction House when creating an auction with stackable items and a stack of the same amount in the inventory. Leak added in 15a2acfb52 .
Fix CreatureAI left uninitialized for 1 server tick for Creatures with DeathState:DEAD . This is a workaround required after c9cf2f059f changes .
Valgrind log:
Conditional jump or move depends on uninitialised value(s)
at : npc_draenei_survivor::npc_draenei_survivorAI::MoveInLineOfSight(Unit*) (zone_azuremyst_isle.cpp:104)
by : CreatureAI::MoveInLineOfSight_Safe(Unit*) (CreatureAI.cpp:123)
by : CreatureUnitRelocationWorker(Creature*, Unit*) (GridNotifiers.cpp:134)
by : Trinity::AIRelocationNotifier::Visit(GridRefManager<Creature>&) (GridNotifiers.cpp:252)
Uninitialised value was created by a heap allocation
at : operator new(unsigned long) (vg_replace_malloc.c:319)
by : npc_draenei_survivor::GetAI(Creature*) const (zone_azuremyst_isle.cpp:66)
by : ScriptMgr::GetCreatureAI(Creature*) (ScriptMgr.cpp:792)
by : FactorySelector::selectAI(Creature*) (CreatureAISelector.cpp:41)
by : Creature::AIM_Initialize(CreatureAI*) (Creature.cpp:716)
by : Creature::AddToWorld() (Creature.cpp:190)
Fix some static analysis issues about uninitialized values. Most of them are false positives, always initialized before being accessed, while some of them are real issues spotted by valgrind too.
Fix the stack implementation used in MotionMaster and added few sanity checks to ensure no underflows will be made.
Valgrind log:
Invalid read of size 8
at : MotionMaster::top() const (MotionMaster.h:115)
by : MotionMaster::pop() (MotionMaster.h:91)
by : MotionMaster::~MotionMaster() (MotionMaster.cpp:74)
by : Unit::~Unit() (Unit.cpp:296)
by : Player::~Player() (Player.cpp:880)
by : WorldSession::HandleCharCreateCallback(Trinity::AutoPtr<PreparedResultSet, ACE_Thread_Mutex>, CharacterCreateInfo*) (CharacterHandler.cpp:665)
by : WorldSession::HandleCharCreateCallback(Trinity::AutoPtr<PreparedResultSet, ACE_Thread_Mutex>, CharacterCreateInfo*) (CharacterHandler.cpp:516)
Fix NULL dereference crash in .npc set link happening when passing a not-existent Creature guid as parameter, es. ".npc set link -1" .
Issue spotted by static analysis, added in c05ed659cb
Fix some static analysis issues about missing NULL checks.
Pass safe Creature pointer instead of summoning a Creature, passing the GUID and retrieve the same Creature from GUID.
Fix some static analysis issues, mostly false positive about fields not initialized in the constructor. It's good practice anyway to always initialize them.
Properly save transport related data to database when saving a Player .
On Player login update the current position as relative to transport, in case the transport moved.
Fix a database race condition between authserver saving session key to database asynchronously and worldserver reading it; session key is now saved synchronously.
Include a Helgrind suppressions file under source control. Each suppressed race is either a false positive, a read race followed by a lock acquire anyway, a bool race. Suppressions related to the same issue in the same function are grouped with a counter in brackets [ ] .
Fix tabs added in bcee801e4b .
Helgrind log:
Lock at 0x2BD582E0 was first observed
at : pthread_mutex_init (hg_intercepts.c:443)
by : ACE_OS::mutex_init(pthread_mutex_t*, int, char const*, pthread_mutexattr_t*, int, int) (in /usr/lib/libACE-6.0.3.so)
by : ACE_Thread_Mutex::ACE_Thread_Mutex(char const*, pthread_mutexattr_t*) (in /usr/lib/libACE-6.0.3.so)
by : WorldSocket::WorldSocket() (WorldSocket.cpp:106)
by : ACE_Acceptor<WorldSocket, ACE_SOCK_Acceptor>::make_svc_handler(WorldSocket*&) (Acceptor.cpp:261)
by : ACE_Acceptor<WorldSocket, ACE_SOCK_Acceptor>::handle_input(int) (Acceptor.cpp:396)
by : ACE_Dev_Poll_Reactor::dispatch_io_event(ACE_Dev_Poll_Reactor::Token_Guard&) (in /usr/lib/libACE-6.0.3.so)
by : ACE_Dev_Poll_Reactor::handle_events(ACE_Time_Value*) (in /usr/lib/libACE-6.0.3.so)
by : ACE_Reactor::run_reactor_event_loop(ACE_Time_Value&, int (*)(ACE_Reactor*)) (in /usr/lib/libACE-6.0.3.so)
by : ReactorRunnable::svc() (WorldSocketMgr.cpp:170)
by : ACE_Task_Base::svc_run(void*) (in /usr/lib/libACE-6.0.3.so)
by : ACE_Thread_Adapter::invoke_i() (in /usr/lib/libACE-6.0.3.so)
Possible data race during write of size 8 at 0x2BD6F6F8 by thread #12
Locks held: 1, at address 0x2BD582E0
at : WorldSession::ResetTimeOutTime() (WorldSession.h:372)
by : WorldSocket::ProcessIncoming(WorldPacket*) (WorldSocket.cpp:709)
by : WorldSocket::handle_input_payload() (WorldSocket.cpp:517)
by : WorldSocket::handle_input_missing_data() (WorldSocket.cpp:610)
by : WorldSocket::handle_input(int) (WorldSocket.cpp:282)
by : ACE_Dev_Poll_Reactor::dispatch_io_event(ACE_Dev_Poll_Reactor::Token_Guard&) (in /usr/lib/libACE-6.0.3.so)
by : ACE_Dev_Poll_Reactor::handle_events(ACE_Time_Value*) (in /usr/lib/libACE-6.0.3.so)
by : ACE_Reactor::run_reactor_event_loop(ACE_Time_Value&, int (*)(ACE_Reactor*)) (in /usr/lib/libACE-6.0.3.so)
by : ReactorRunnable::svc() (WorldSocketMgr.cpp:170)
by : ACE_Task_Base::svc_run(void*) (in /usr/lib/libACE-6.0.3.so)
by : ACE_Thread_Adapter::invoke_i() (in /usr/lib/libACE-6.0.3.so)
by : ACE_Thread_Adapter::invoke() (in /usr/lib/libACE-6.0.3.so)
This conflicts with a previous write of size 8 by thread #7
Locks held: none
at : WorldSession::UpdateTimeOutTime(unsigned int) (WorldSession.h:368)
by : WorldSession::Update(unsigned int, PacketFilter&) (WorldSession.cpp:256)
by : World::UpdateSessions(unsigned int) (World.cpp:2646)
by : World::Update(unsigned int) (World.cpp:2003)
by : WorldRunnable::run() (WorldRunnable.cpp:60)
by : ACE_Based::Thread::ThreadTask(void*) (Threading.cpp:186)
by : ACE_OS_Thread_Adapter::invoke() (in /usr/lib/libACE-6.0.3.so)
by : mythread_wrapper (hg_intercepts.c:233)
Add additional assert to assure that helgrind reports related to SQLQueryHolderTask::Execute() and WorldSession::ProcessQueryCallbacks() are false positives.
The only chance for WorldSession::ProcessQueryCallbacks() to access the SQL queries before they are completed is if m_result.ready() is true before the end of SQLQueryHolderTask::Execute() .
Helgrind false positive log:
Possible data race during read of size 8 at 0x2BD641E0 by thread #7
Locks held: none
at : SQLQueryHolder::GetPreparedResult(unsigned long) (QueryHolder.cpp:107)
by : Player::LoadFromDB(unsigned int, SQLQueryHolder*) (Player.cpp:16917)
by : WorldSession::HandlePlayerLogin(LoginQueryHolder*) (CharacterHandler.cpp:807)
by : WorldSession::ProcessQueryCallbacks() (WorldSession.cpp:1112)
by : WorldSession::Update(unsigned int, PacketFilter&) (WorldSession.cpp:401)
by : World::UpdateSessions(unsigned int) (World.cpp:2646)
by : World::Update(unsigned int) (World.cpp:2003)
by : WorldRunnable::run() (WorldRunnable.cpp:60)
by : ACE_Based::Thread::ThreadTask(void*) (Threading.cpp:186)
by : ACE_OS_Thread_Adapter::invoke() (in /usr/lib/libACE-6.0.3.so)
by : mythread_wrapper (hg_intercepts.c:233)
by : start_thread (pthread_create.c:311)
This conflicts with a previous write of size 8 by thread #4
Locks held: none
at : SQLQueryHolder::SetPreparedResult(unsigned long, PreparedResultSet*) (QueryHolder.cpp:140)
by : SQLQueryHolderTask::Execute() (QueryHolder.cpp:196)
by : SQLOperation::call() (SQLOperation.h:65)
by : DatabaseWorker::svc() (DatabaseWorker.cpp:45)
by : ACE_Task_Base::svc_run(void*) (in /usr/lib/libACE-6.0.3.so)
by : ACE_Thread_Adapter::invoke_i() (in /usr/lib/libACE-6.0.3.so)
by : ACE_Thread_Adapter::invoke() (in /usr/lib/libACE-6.0.3.so)
by : mythread_wrapper (hg_intercepts.c:233)
Address 0x2BD641E0 is 16 bytes inside a block of size 792 alloc'd
at : operator new(unsigned long) (vg_replace_malloc.c:319)
by : __gnu_cxx::new_allocator<std::pair<SQLElementData, SQLResultSetUnion> >::allocate(unsigned long, void const*) (new_allocator.h:104)
by : std::_Vector_base<std::pair<SQLElementData, SQLResultSetUnion>, std::allocator<std::pair<SQLElementData, SQLResultSetUnion> > >::_M_allocate(unsigned long) (in /home/jackpoz/trinity/bin/worldserver)
by : std::vector<std::pair<SQLElementData, SQLResultSetUnion>, std::allocator<std::pair<SQLElementData, SQLResultSetUnion> > >::_M_fill_insert(__gnu_cxx::__normal_iterator<std::pair<SQLElementData, SQLResultSetUnion>*, std::vector<std::pair<SQLElementData, SQLResultSetUnion>, std::allocator<std::pair<SQLElementData, SQLResultSetUnion> > > >, unsigned long, std::pair<SQLElementData, SQLResultSetUnion> const&) (vector.tcc:483)
by : std::vector<std::pair<SQLElementData, SQLResultSetUnion>, std::allocator<std::pair<SQLElementData, SQLResultSetUnion> > >::insert(__gnu_cxx::__normal_iterator<std::pair<SQLElementData, SQLResultSetUnion>*, std::vector<std::pair<SQLElementData, SQLResultSetUnion>, std::allocator<std::pair<SQLElementData, SQLResultSetUnion> > > >, unsigned long, std::pair<SQLElementData, SQLResultSetUnion> const&) (stl_vector.h:1024)
by : std::vector<std::pair<SQLElementData, SQLResultSetUnion>, std::allocator<std::pair<SQLElementData, SQLResultSetUnion> > >::resize(unsigned long, std::pair<SQLElementData, SQLResultSetUnion>) (stl_vector.h:707)
by : SQLQueryHolder::SetSize(unsigned long) (QueryHolder.cpp:167)
by : LoginQueryHolder::Initialize() (CharacterHandler.cpp:66)
by : WorldSession::HandlePlayerLoginOpcode(WorldPacket&) (CharacterHandler.cpp:788)
by : WorldSession::Update(unsigned int, PacketFilter&) (WorldSession.cpp:363)
by : World::UpdateSessions(unsigned int) (World.cpp:2646)
by : World::Update(unsigned int) (World.cpp:2003)
Allow to add overlapping teles like "abc" even if "abcde" already exists. This wasn't possible and a "Teleport already exists" error was thrown instead.
Fix uninitialized Docks timer in Isle Of Conquest .
Valgrind log:
Conditional jump or move depends on uninitialised value(s)
at : BattlegroundIC::HandleCapturedNodes(ICNodePoint*, bool) (BattlegroundIC.cpp:665)
by : BattlegroundIC::PostUpdateImpl(unsigned int) (BattlegroundIC.cpp:228)
by : Battleground::Update(unsigned int) (Battleground.cpp:300)
by : BattlegroundMgr::Update(unsigned int) (BattlegroundMgr.cpp:93)
by : World::Update(unsigned int) (World.cpp:2062)
Remove assertion about Quest ExclusiveGroup since they are triggered even when adding valid quests to npcs. Fixes#8336 .
How to reproduce steps:
- .gm off
- add quest 11335 in creature_queststarter
- .reload creature_queststarter
- get in range of that creature
- .gm on
- assertion triggered
Mitigate DoS attacks like one explained in #10555 by processing a limited number of packets at each WorldSession::Update() call, allowing other WorldSessions to be processed.
100 packets in a single update sound like a reasonable amount.
Mitigate DoS attacks to authserver like "Wow Auth Flooder.exe" by allowing a finite number of AUTH_LOGON_CHALLEGE packets in a row from same socket, 3 sounds like a reasonable limit.
Don't ignore USE_SCRIPTPCH cmake flag if SCRIPTS is set to FALSE; in this case "script" project has only source files related to scripted spells and command scripts and it must be built with precompiled headers if USE_SCRIPTPCH flag is set. This partially reverts 780ed1b01c .
Fix not-NULL terminated char buffer and uninitialized memory written to vmaps due to WMOLiquidHeader struct padding bytes left uninitialized. The padding bytes should be ignored anyway by VMAP Assembler so no re-extraction is required.
Valgrind log:
Syscall param write(buf) points to uninitialised byte(s)
at 0x585FB50: __write_nocancel (syscall-template.S:81)
by 0x57EE2F2: _IO_file_write@@GLIBC_2.2.5 (fileops.c:1270)
by 0x57EE1D1: new_do_write (fileops.c:546)
by 0x57EF904: _IO_do_write@@GLIBC_2.2.5 (fileops.c:519)
by 0x57EEB70: _IO_file_xsputn@@GLIBC_2.2.5 (fileops.c:1341)
by 0x57E3EC4: fwrite (iofwrite.c:43)
by 0x41161B: WMOGroup::ConvertToVMAPGroupWmo(_IO_FILE*, WMORoot*, bool) (wmo.cpp:386)
by 0x407FA9: ExtractSingleWmo(std::string&) (vmapexport.cpp:215)
by 0x407B16: ExtractWmo() (vmapexport.cpp:138)
by 0x409D50: main (vmapexport.cpp:497)
Address 0x40398e2 is not stack'd, malloc'd or (recently) free'd
Uninitialised value was created by a heap allocation
at 0x4C2C7A7: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x410ADA: WMOGroup::open() (wmo.cpp:224)
by 0x407F54: ExtractSingleWmo(std::string&) (vmapexport.cpp:208)
by 0x407B16: ExtractWmo() (vmapexport.cpp:138)
by 0x409D50: main (vmapexport.cpp:497)
Fix not-NULL terminated char buffers and char[] to string conversion.
Valgrind logs:
Invalid read of size 1
at 0x4C2D7D4: __GI_strlen (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x5318BAF: std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.17)
by 0x409471: ReadBuild(int) (System.cpp:189)
by 0x40CEC6: main (System.cpp:1108)
Address 0x5f851ee is 0 bytes after a block of size 94 alloc'd
at 0x4C2C037: operator new[](unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x40F63A: MPQFile::MPQFile(char const*) (mpq_libmpq.cpp:65)
by 0x409408: ReadBuild(int) (System.cpp:182)
by 0x40CEC6: main (System.cpp:1108)
Invalid read of size 1
at 0x57FFCAC: strtok (strtok.S:165)
by 0x40D279: MPQArchive::GetFileListTo(std::vector<std::string, std::allocator<std::string> >&) (mpq_libmpq04.h:45)
by 0x40C5CF: ExtractDBCFiles(int, bool) (System.cpp:1001)
by 0x40CEF5: main (System.cpp:1110)
Address 0x5f91e27 is 0 bytes after a block of size 44,391 alloc'd
at 0x4C2C037: operator new[](unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x40D1A5: MPQArchive::GetFileListTo(std::vector<std::string, std::allocator<std::string> >&) (mpq_libmpq04.h:30)
by 0x40C5CF: ExtractDBCFiles(int, bool) (System.cpp:1001)
by 0x40CEF5: main (System.cpp:1110)
Valgrind log:
Conditional jump or move depends on uninitialised value(s)
at 0x1798C2C: npc_medivh_bm::npc_medivh_bmAI::UpdateAI(unsigned int) (the_black_morass.cpp:192)
by 0x11AEFC0: Creature::Update(unsigned int) (Creature.cpp:542)
by 0x12E96F8: Trinity::ObjectUpdater::Visit(GridRefManager<Creature>&) (GridNotifiersImpl.h:45)
Conditional jump or move depends on uninitialised value(s)
at 0x1798C7E: npc_medivh_bm::npc_medivh_bmAI::UpdateAI(unsigned int) (the_black_morass.cpp:200)
by 0x11AEFC0: Creature::Update(unsigned int) (Creature.cpp:542)
by 0x12E96F8: Trinity::ObjectUpdater::Visit(GridRefManager<Creature>&) (GridNotifiersImpl.h:45)
Conditional jump or move depends on uninitialised value(s)
at 0x1798CB0: npc_medivh_bm::npc_medivh_bmAI::UpdateAI(unsigned int) (the_black_morass.cpp:205)
by 0x11AEFC0: Creature::Update(unsigned int) (Creature.cpp:542)
by 0x12E96F8: Trinity::ObjectUpdater::Visit(GridRefManager<Creature>&) (GridNotifiersImpl.h:45)
Conditional jump or move depends on uninitialised value(s)
at 0x1798CE2: npc_medivh_bm::npc_medivh_bmAI::UpdateAI(unsigned int) (the_black_morass.cpp:210)
by 0x11AEFC0: Creature::Update(unsigned int) (Creature.cpp:542)
by 0x12E96F8: Trinity::ObjectUpdater::Visit(GridRefManager<Creature>&) (GridNotifiersImpl.h:45)
Port some movement packet fixes, taking into account client/server connection lag.
Credits to original author, check http://sourceforge.net/p/ascentemubackup/code/1637/ for more details.
MOVEMENT_PACKET_TIME_DELAY default value is currently set to 0, feel free to set it to different values and report feedback.
Proper initialize POD struct members to 0 in SMART_ACTION_CREATE_TIMED_EVENT action type.
Fix array overflows and access uninitialized values in SMART_ACTION_CALL_RANDOM_TIMED_ACTIONLIST, SMART_ACTION_RANDOM_PHASE and SMART_ACTION_RANDOM_EMOTE .
Valgrind log for SMART_ACTION_RANDOM_PHASE case:
Conditional jump or move depends on uninitialised value(s)
at : SmartScript::UpdateTimer(SmartScriptHolder&, unsigned int) (SmartScript.cpp:3086)
by : SmartScript::OnUpdate(unsigned int) (SmartScript.cpp:3176)
by : SmartAI::UpdateAI(unsigned int) (SmartAI.cpp:335)
by : Creature::Update(unsigned int) (Creature.cpp:542)
by : TempSummon::Update(unsigned int) (TemporarySummon.cpp:47)
Uninitialised value was created by a stack allocation
at : SmartScript::ProcessAction(SmartScriptHolder&, Unit*, unsigned int, unsigned int, bool, SpellInfo const*, GameObject*) (SmartScript.cpp:143)
Reset() has if checks on values that are initialized after them so it's required to initialize these fields in the constructor.
Valgrind log:
Conditional jump or move depends on uninitialised value(s)
by 0x11CF472: HashMapHolder<Creature>::Find(unsigned long) (ObjectAccessor.h:67)
by 0x11CE36F: Creature* ObjectAccessor::GetObjectInWorld<Creature>(unsigned long, Creature*) (ObjectAccessor.h:114)
by 0x1257C13: Creature* ObjectAccessor::GetObjectInMap<Creature>(unsigned long, Map*, Creature*) (ObjectAccessor.h:135)
by 0x12E64F1: Map::GetCreature(unsigned long) (Map.cpp:2767)
by 0x10AF4DE: Unit::GetCreature(WorldObject&, unsigned long) (Unit.cpp:13021)
by 0x16F8F81: npc_highlord_darion_mograine::npc_highlord_darion_mograineAI::Reset() (chapter5.cpp:386)
by 0x16F8D36: npc_highlord_darion_mograine::npc_highlord_darion_mograineAI::npc_highlord_darion_mograineAI(Creature*) (chapter5.cpp:314)
by 0x16F8CD6: npc_highlord_darion_mograine::GetAI(Creature*) const (chapter5.cpp:307)
by 0x132AF6C: ScriptMgr::GetCreatureAI(Creature*) (ScriptMgr.cpp:796)
by 0x1441D45: FactorySelector::selectAI(Creature*) (CreatureAISelector.cpp:41)
Change DatabaseWorkerPool<T>::DirectExecute() to handle PreparedStatement disposal in the same way of DatabaseWorkerPool<T>::Query() , thanks Joschiwald for patch.
Fix .account create command not deleting a MySql PreparedStatement .
Valgrind log:
254 (40 direct, 214 indirect) bytes in 1 blocks are definitely lost in loss record 6 of 8
at 0x4C24DFA: operator new(unsigned long) (vg_replace_malloc.c:261)
by 0x998EC2: DatabaseWorkerPool<LoginDatabaseConnection>::GetPreparedStatement(unsigned int) (DatabaseWorkerPool.h:437)
by 0x99599B: AccountMgr::CreateAccount(std::string, std::string, std::string) (AccountMgr.cpp:49)
by 0xF3ABAC: account_commandscript::HandleAccountCreateCommand(ChatHandler*, char const*)
Fix conditional jump on uninitialized value in PathGenerator::GetNavTerrain() by checking the result of getLiquidStatus() before accessing the LiquidData, uninitialized in case there's no water at all.
Valgrind log:
Conditional jump or move depends on uninitialised value(s)
at : PathGenerator::GetNavTerrain(float, float, float) (PathGenerator.cpp:553)
by : PathGenerator::UpdateFilter() (PathGenerator.cpp:542)
by : PathGenerator::CreateFilter() (PathGenerator.cpp:530)
by : PathGenerator::PathGenerator(Unit const*) (PathGenerator.cpp:46)
by : Spell::Spell(Unit*, SpellInfo const*, TriggerCastFlags, unsigned long, bool) (Spell.cpp:493)
by : Unit::CastSpell(SpellCastTargets const&, SpellInfo const*, CustomSpellValues const*, TriggerCastFlags, Item*, AuraEffect const*, unsigned long) (Unit.cpp:846)
by : Unit::CastSpell(Unit*, SpellInfo const*, TriggerCastFlags, Item*, AuraEffect const*, unsigned long) (Unit.cpp:882)
by : Unit::CastSpell(Unit*, unsigned int, TriggerCastFlags, Item*, AuraEffect const*, unsigned long) (Unit.cpp:870)
by : Unit::CastSpell(Unit*, unsigned int, bool, Item*, AuraEffect const*, unsigned long) (Unit.cpp:858)
by : Player::SendInitialPacketsAfterAddToMap() (Player.cpp:22580)
by : WorldSession::HandleMoveWorldportAckOpcode() (MovementHandler.cpp:121)
by : WorldSession::HandleMoveWorldportAckOpcode(WorldPacket&) (MovementHandler.cpp:37)
Restore 2 disabled warnings, 1 already disabled in CMake with the WITH_WARNINGS flag set to False and the other disabled by default as stated in VS documentation http://msdn.microsoft.com/en-us/library/aa984150.aspx
Fix race condition in Log by using atomic operators.
Helgrind log:
Possible data race during read of size 8 at 0x7379D98 by thread #1
Locks held: none
at 0x15AE9C7: AppenderFile::_write(LogMessage const&) (AppenderFile.cpp:59)
by 0x15ADFF8: Appender::write(LogMessage&) (Appender.cpp:106)
by 0x159F14E: Logger::write(LogMessage&) (Logger.cpp:83)
by 0x15A215B: Log::write(LogMessage*) (Log.cpp:290)
by 0x15A200F: Log::vlog(LogFilterType, LogLevel, char const*, __va_list_tag*) (Log.cpp:272)
by 0x15A2682: Log::outInfo(LogFilterType, char const*, ...) (Log.cpp:364)
by 0xF7DA28: Master::Run() (Master.cpp:296)
by 0xF835E8: main (Main.cpp:142)
This conflicts with a previous write of size 8 by thread #10
Locks held: none
at 0x15AE9D7: AppenderFile::_write(LogMessage const&) (AppenderFile.cpp:59)
by 0x15ADFF8: Appender::write(LogMessage&) (Appender.cpp:106)
by 0x159F14E: Logger::write(LogMessage&) (Logger.cpp:83)
by 0x15A215B: Log::write(LogMessage*) (Log.cpp:290)
by 0x15A200F: Log::vlog(LogFilterType, LogLevel, char const*, __va_list_tag*) (Log.cpp:272)
by 0x15A2682: Log::outInfo(LogFilterType, char const*, ...) (Log.cpp:364)
by 0xF7EC1F: FreezeDetectorRunnable::run() (Master.cpp:98)
by 0x15A5B3E: ACE_Based::Thread::ThreadTask(void*) (Threading.cpp:186)
Address 0x7379D98 is 88 bytes inside a block of size 96 alloc'd
at 0x4C2C857: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
by 0x15A0FE3: Log::CreateAppenderFromConfig(char const*) (Log.cpp:150)
by 0x15A1AED: Log::ReadAppendersFromConfig() (Log.cpp:244)
by 0x15A31B4: Log::LoadFromConfig() (Log.cpp:469)
by 0x15A053B: Log::Log() (Log.cpp:35)
by 0xF75CD1: ACE_Singleton<Log, ACE_Thread_Mutex>::ACE_Singleton() (Singleton.inl:13)
by 0xF754A4: ACE_Singleton<Log, ACE_Thread_Mutex>::instance() (Singleton.cpp:91)
by 0xF8351A: main (Main.cpp:135)
Change IntervalTimer::Reset() behavior to handle system clock changes forward and backward.
This fixes IntervalTimer:.Passed() returning true till it catches up to the new time, triggering the event up to "std::numeric_limits<time_t>::max() / _interval" times.
Fixes https://github.com/TrinityCore/TrinityCore/issues/5816
Replace thread-unsafe localtime() http://www.cplusplus.com/reference/ctime/localtime/ with thread-safe portable ACE_OS::localtime_r() .
Helgrind log:
Possible data race during read of size 4 at 0x6F183C0 by thread #1
Locks held: none
at 0x14E72E3: World::InitDailyQuestResetTime() (World.cpp:2772)
by 0x14E3A01: World::SetInitialWorldSettings() (World.cpp:1790)
by 0x101122A: Master::Run() (Master.cpp:164)
by 0x101740C: main (Main.cpp:142)
This conflicts with a previous write of size 4 by thread #2
Locks held: none
at 0x6C2D3BA: __tzfile_compute (tzfile.c:797)
by 0x6C2D036: __tz_convert (tzset.c:627)
by 0x164146C: LogMessage::getTimeStr(long) (Appender.cpp:23)
by 0x1641550: LogMessage::getTimeStr() (Appender.cpp:31)
by 0x1641722: Appender::write(LogMessage&) (Appender.cpp:80)
by 0x1633FCE: Logger::write(LogMessage&) (Logger.cpp:83)
by 0x16433D8: LogOperation::call() (LogOperation.cpp:29)
by 0x16428A4: LogWorker::svc() (LogWorker.cpp:45)
Fix memory leak when disbanding a Guild, removed from GuildMgr but never deleted. Due to the different ways of how Guild::Disband() and Guild::DeleteMember() are called, each call to these methods has a different way to delete the Guild if empty/invalid.
Valgrind log:
2,127 (240 direct, 1,887 indirect) bytes in 1 blocks are definitely lost in loss record 54 of 81
at operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by guild_commandscript::HandleGuildCreateCommand(ChatHandler*, char const*) (cs_guild.cpp:91)
by ChatHandler::ExecuteCommandInTable(ChatCommand*, char const*, std::string const&) (Chat.cpp:339)
by ChatHandler::ExecuteCommandInTable(ChatCommand*, char const*, std::string const&) (Chat.cpp:320)
by ChatHandler::ParseCommands(char const*) (Chat.cpp:466)
by WorldSession::HandleMessagechatOpcode(WorldPacket&) (ChatHandler.cpp:217)
by WorldSession::Update(unsigned int, PacketFilter&) (WorldSession.cpp:317)
by World::UpdateSessions(unsigned int) (World.cpp:2632)
by World::Update(unsigned int) (World.cpp:1989)
by WorldRunnable::run() (WorldRunnable.cpp:60)
by ACE_Based::Thread::ThreadTask(void*) (Threading.cpp:186)
by ACE_OS_Thread_Adapter::invoke() (in /usr/lib/libACE-6.0.3.so)
Partially revert 58ec4e5319 and increment GuildStore iterator before validating Guilds. This is required because Guild::Validate() removes the Guild from GuildStore if invalid.
Closes#8363
Fix memory leak in command line handler on platforms other than Windows. The result of readline() is supposed to be freed with free() as described at http://cnswww.cns.cwru.edu/php/chet/readline/readline.html#SEC24 .
Valgrind log:
11 bytes in 2 blocks are definitely lost in loss record 6 of 61
at 0x4C28BED: malloc (vg_replace_malloc.c:263)
by 0x4E5F6E8: xmalloc (in /lib/x86_64-linux-gnu/libreadline.so.6.2)
by 0x4E4571A: readline_internal_teardown (in /lib/x86_64-linux-gnu/libreadline.so.6.2)
by 0x4E46541: readline (in /lib/x86_64-linux-gnu/libreadline.so.6.2)
by 0x1005284: CliRunnable::run() (CliRunnable.cpp:161)
by 0x163A3DA: ACE_Based::Thread::ThreadTask(void*) (Threading.cpp:186)
by 0x518C555: ACE_OS_Thread_Adapter::invoke() (OS_Thread_Adapter.cpp:103)
by 0x61D7B4F: start_thread (pthread_create.c:304)
by 0x6C66A7C: clone (clone.S:112)
Fix uninitialized fallTime and splineElevation fields of MovementInfo.
Valgrind log:
Conditional jump or move depends on uninitialised value(s)
at 0x5FBEB50: deflate (in /lib/x86_64-linux-gnu/libz.so.1.2.7)
by 0x1269883: UpdateData::Compress(void*, unsigned int*, void*, int) (UpdateData.cpp:85)
by 0x1269BB6: UpdateData::BuildPacket(WorldPacket*) (UpdateData.cpp:133)
by 0x130E9B3: Trinity::VisibleNotifier::SendToSelf() (GridNotifiers.cpp:68)
by 0x117A89D: Player::UpdateVisibilityForPlayer() (Player.cpp:22344)
by 0x117B334: Player::SendInitialPacketsAfterAddToMap() (Player.cpp:22567)
by 0x15A48BC: WorldSession::HandlePlayerLogin(LoginQueryHolder*) (CharacterHandler.cpp:937)
by 0x141D04B: WorldSession::ProcessQueryCallbacks() (WorldSession.cpp:1106)
by 0x141994D: WorldSession::Update(unsigned int, PacketFilter&) (WorldSession.cpp:391)
by 0x14E289C: World::UpdateSessions(unsigned int) (World.cpp:2629)
by 0x14E0613: World::Update(unsigned int) (World.cpp:1986)
by 0x100B37F: WorldRunnable::run() (WorldRunnable.cpp:60)
Uninitialised value was created by a heap allocation
at 0x4C286E7: operator new(unsigned long) (vg_replace_malloc.c:287)
by 0x159E64F: void LoadHelper<Creature>(std::set<unsigned int, std::less<unsigned int>, std::allocator<unsigned int> > const&, CoordPair<512u>&, GridRefManager<Creature>&, unsigned int&, Map*) (ObjectGridLoader.cpp:94)
by 0x159DECF: ObjectGridLoader::Visit(GridRefManager<Creature>&) (ObjectGridLoader.cpp:150)
by 0x159F092: void VisitorHelper<ObjectGridLoader, Creature>(ObjectGridLoader&, ContainerMapList<Creature>&) (TypeContainerVisitor.h:64)
by 0x159EFF5: void VisitorHelper<ObjectGridLoader, Creature, TypeList<DynamicObject, TypeList<Corpse, TypeNull> > >(ObjectGridLoader&, ContainerMapList<TypeList<Creature, TypeList<DynamicObject, TypeList<Corpse, TypeNull> > > >&) (TypeContainerVisitor.h:70)
Partially revert 167ca565d0 which wrongly used TYPEMASK_UNIT to denote Creatures and Players, so when a Player sent farsight opcode with another Player as target HandleFarSightOpcode() couldn't find the right Unit filtering by typemask TYPEMASK_SEER.
ReactorRunnable::svc() checks the status of WorldSockets while WorldRunnable::run() updates them, causing a race condition.
Helgrind log:
Possible data race during write of size 8 at 0x49961810 by thread #9
Locks held: 1, at address 0x4367A620
at 0x51781F7: ACE_Message_Block::copy(char const*, unsigned long) (Message_Block.inl:372)
by 0x15D048F: WorldSocket::SendPacket(WorldPacket const&) (WorldSocket.cpp:180)
by 0x141C45E: WorldSession::SendPacket(WorldPacket const*) (WorldSession.cpp:223)
by 0x15C49C4: WorldSession::SendAuthResponse(unsigned char, bool, unsigned int) (AuthHandler.cpp:37)
by 0x14DA71C: World::AddSession_(WorldSession*) (World.cpp:278)
by 0x14E601E: World::UpdateSessions(unsigned int) (World.cpp:2617)
by 0x14E3E67: World::Update(unsigned int) (World.cpp:1986)
by 0x100EAFA: WorldRunnable::run() (WorldRunnable.cpp:60)
by 0x163A626: ACE_Based::Thread::ThreadTask(void*) (Threading.cpp:186)
by 0x518F555: ACE_OS_Thread_Adapter::invoke() (OS_Thread_Adapter.cpp:103)
by 0x4C2B5AD: mythread_wrapper (hg_intercepts.c:219)
by 0x61DAB4F: start_thread (pthread_create.c:304)
This conflicts with a previous read of size 8 by thread #14
Locks held: none
at 0x1008414: ACE_Message_Block::length() const (Message_Block.inl:131)
by 0x15D1207: WorldSocket::Update() (WorldSocket.cpp:448)
by 0x1427CA3: ReactorRunnable::svc() (WorldSocketMgr.cpp:177)
by 0x51CBB16: ACE_Task_Base::svc_run(void*) (Task.cpp:271)
by 0x51CD3BC: ACE_Thread_Adapter::invoke_i() (Thread_Adapter.cpp:161)
by 0x51CD4D4: ACE_Thread_Adapter::invoke() (Thread_Adapter.cpp:96)
by 0x4C2B5AD: mythread_wrapper (hg_intercepts.c:219)
by 0x61DAB4F: start_thread (pthread_create.c:304)
Address 0x49961810 is 16 bytes inside a block of size 80 alloc'd
at 0x4C286BB: operator new(unsigned long, std::nothrow_t const&) (vg_replace_malloc.c:316)
by 0x15D0818: WorldSocket::open(void*) (WorldSocket.cpp:237)
by 0x1429560: ACE_Acceptor<WorldSocket, ACE_SOCK_Acceptor>::activate_svc_handler(WorldSocket*) (Acceptor.cpp:347)
by 0x142916D: ACE_Acceptor<WorldSocket, ACE_SOCK_Acceptor>::handle_input(int) (Acceptor.cpp:429)
by 0x515F48D: ACE_Dev_Poll_Reactor::dispatch_io_event(ACE_Dev_Poll_Reactor::Token_Guard&) (Dev_Poll_Reactor.inl:86)
by 0x5161653: ACE_Dev_Poll_Reactor::handle_events(ACE_Time_Value*) (Dev_Poll_Reactor.cpp:1015)
by 0x51ACCFC: ACE_Reactor::run_reactor_event_loop(ACE_Time_Value&, int (*)(ACE_Reactor*)) (Reactor.cpp:267)
by 0x1427C57: ReactorRunnable::svc() (WorldSocketMgr.cpp:170)
by 0x51CBB16: ACE_Task_Base::svc_run(void*) (Task.cpp:271)
by 0x51CD3BC: ACE_Thread_Adapter::invoke_i() (Thread_Adapter.cpp:161)
by 0x51CD4D4: ACE_Thread_Adapter::invoke() (Thread_Adapter.cpp:96)
by 0x4C2B5AD: mythread_wrapper (hg_intercepts.c:219)
Fix race condition by replacing a static volatile uint32 with proper atomic thread-safe ACE_Atomic_Op<ACE_Thread_Mutex, uint32>, incremented in WorldRunnable::run() at each world loop and read in FreezeDetectorRunnable::run().
Helgrind log:
Possible data race during read of size 4 at 0x2400D54 by thread #12
Locks held: none
at 0x100FEA6: FreezeDetectorRunnable::run() (Master.cpp:106)
by 0x1637892: ACE_Based::Thread::ThreadTask(void*) (Threading.cpp:186)
by 0x518F555: ACE_OS_Thread_Adapter::invoke() (OS_Thread_Adapter.cpp:103)
by 0x4C2B5AD: mythread_wrapper (hg_intercepts.c:219)
by 0x61DAB4F: start_thread (pthread_create.c:304)
by 0x6C69A7C: clone (clone.S:112)
This conflicts with a previous write of size 4 by thread #9
Locks held: none
at 0x100C23E: WorldRunnable::run() (WorldRunnable.cpp:55)
by 0x1637892: ACE_Based::Thread::ThreadTask(void*) (Threading.cpp:186)
by 0x518F555: ACE_OS_Thread_Adapter::invoke() (OS_Thread_Adapter.cpp:103)
by 0x4C2B5AD: mythread_wrapper (hg_intercepts.c:219)
by 0x61DAB4F: start_thread (pthread_create.c:304)
by 0x6C69A7C: clone (clone.S:112)
Fix an assert triggered by float to int32 cast overflowing to -1, now it replaces -1 with max int32 value.
Fix another assert triggered by Vector3 magnitude float overflow to max/infinity in StaticMapTree::isInLineOfSight(), in this case return false.
Both asserts can be reproduced by casting Mind Control to a NPC, tele to z: 1.0e+38 using client hack tools, move to allow the server to register the new position and stop Mind Control.
Closes#8970Closes#10578Closes#10355Closes#10673
Mind Vision has Aura Effect SPELL_AURA_BIND_SIGHT which adds the target to a special Map container i_objectsToSwitch, used to switch grid containers for target Creatures of this Aura Effect.
When the target is a Creature, when the Creature is removed from world it's added to i_objectsToSwitch and then to i_objectsToRemove, iterated in this order in Map::RemoveAllObjectsInRemoveList() so the reference in i_objectsToSwitch is valid.
When the target is a Player, when the Player logs out it's added to i_objectsToSwitch but then Map::RemovePlayerFromMap() deletes the Player, leaving an invalid reference in i_objectsToSwitch.
Since the whole point of i_objectsToSwitch is to store Creatures and since the stored references are used only if the condition "GetTypeId() == TYPEID_UNIT" is verified, it's safe to add only objects of TYPEID_UNIT type to the container.
Valgrind log:
Invalid read of size 4
at 0xC52332: Object::GetTypeId() const (Object.h:140)
by 0xF540D3: Map::RemoveAllObjectsInRemoveList() (Map.cpp:2136)
by 0xF53CD2: Map::DelayedUpdate(unsigned int) (Map.cpp:2087)
by 0xF639B1: MapManager::Update(unsigned int) (MapManager.cpp:292)
by 0x107CB40: World::Update(unsigned int) (World.cpp:2025)
by 0xBEB263: WorldRunnable::run() (WorldRunnable.cpp:60)
by 0x1213792: ACE_Based::Thread::ThreadTask(void*) (Threading.cpp:186)
by 0x515EA35: ACE_OS_Thread_Adapter::invoke() (in /usr/lib/libACE-6.0.3.so)
by 0x5F19F8D: start_thread (pthread_create.c:311)
by 0x6A46E1C: clone (clone.S:113)
Address 0x401eacac is 12 bytes inside a block of size 11,736 free'd
at 0x4C2B59C: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0xD80239: Player::~Player() (Player.cpp:915)
by 0xF4D5A2: void Map::DeleteFromWorld<Player>(Player*) (Map.cpp:319)
by 0xF4EBBB: Map::RemovePlayerFromMap(Player*, bool) (Map.cpp:687)
by 0xFCC18D: WorldSession::LogoutPlayer(bool) (WorldSession.cpp:531)
by 0xF1EDD5: WorldSession::HandleLogoutRequestOpcode(WorldPacket&) (MiscHandler.cpp:403)
by 0xFCAE37: WorldSession::Update(unsigned int, PacketFilter&) (WorldSession.cpp:312)
by 0x107EBC6: World::UpdateSessions(unsigned int) (World.cpp:2615)
by 0x107C94B: World::Update(unsigned int) (World.cpp:1978)
by 0xBEB263: WorldRunnable::run() (WorldRunnable.cpp:60)
by 0x1213792: ACE_Based::Thread::ThreadTask(void*) (Threading.cpp:186)
by 0x515EA35: ACE_OS_Thread_Adapter::invoke() (in /usr/lib/libACE-6.0.3.so)
Fix GuildMember flags left uninitialized when creating a new guild.
Valgrind log:
Conditional jump or move depends on uninitialised value(s)
at 0xEE49CE: Guild::Member::WritePacket(WorldPacket&) const (Guild.cpp:714)
by 0xEE721E: Guild::HandleRoster(WorldSession*) (Guild.cpp:1314)
by 0xEE9EBA: Guild::SendLoginInfo(WorldSession*) (Guild.cpp:1920)
by 0xEEB689: Guild::AddMember(unsigned long, unsigned char) (Guild.cpp:2230)
by 0xEE6A09: Guild::Create(Player*, std::string const&) (Guild.cpp:1199)
by 0x12A71F9: guild_commandscript::HandleGuildCreateCommand(ChatHandler*, char const*) (cs_guild.cpp:92)
by 0xC47DBE: ChatHandler::ExecuteCommandInTable(ChatCommand*, char const*, std::string const&) (Chat.cpp:362)
by 0xC47C23: ChatHandler::ExecuteCommandInTable(ChatCommand*, char const*, std::string const&) (Chat.cpp:343)
by 0xC488C4: ChatHandler::ParseCommands(char const*) (Chat.cpp:489)
by 0x1188EE3: WorldSession::HandleMessagechatOpcode(WorldPacket&) (ChatHandler.cpp:217)
by 0xFCAE37: WorldSession::Update(unsigned int, PacketFilter&) (WorldSession.cpp:312)
by 0x107EBC6: World::UpdateSessions(unsigned int) (World.cpp:2615)
Modify how InstanceSave is deleted so the local mutex can be released before deleting the class itself.
Valgrind log:
Invalid read of size 4
at 0x662662B: __pthread_mutex_unlock_usercnt (pthread_mutex_unlock.c:52)
by 0x55D3C55: ACE_OS::mutex_unlock(pthread_mutex_t*) (OS_NS_Thread.cpp:2335)
by 0xB20057: Player::CleanupsBeforeDelete(bool) (OS_NS_Thread.inl:3519)
by 0xD0E2FA: WorldSession::LogoutPlayer(bool) (WorldSession.cpp:527)
by 0xC66D34: WorldSession::HandleLogoutRequestOpcode(WorldPacket&) (MiscHandler.cpp:403)
by 0xD0EA82: WorldSession::Update(unsigned int, PacketFilter&) (WorldSession.cpp:312)
by 0xD9AD66: World::UpdateSessions(unsigned int) (World.cpp:2615)
by 0xD9BEC4: World::Update(unsigned int) (World.cpp:1978)
by 0xA035E5: WorldRunnable::run() (WorldRunnable.cpp:60)
by 0xEC8D39: ACE_Based::Thread::ThreadTask(void*) (Threading.cpp:183)
by 0x55D7555: ACE_OS_Thread_Adapter::invoke() (OS_Thread_Adapter.cpp:103)
by 0x6622B4F: start_thread (pthread_create.c:304)
Address 0x1884bb08 is 56 bytes inside a block of size 104 free'd
at 0x4C279DC: operator delete(void*) (vg_replace_malloc.c:457)
by 0xC9D533: InstanceSaveManager::RemoveInstanceSave(unsigned int) (InstanceSaveMgr.cpp:159)
by 0xC9E826: InstanceSave::UnloadIfEmpty() (InstanceSaveMgr.cpp:238)
by 0xB2003E: Player::CleanupsBeforeDelete(bool) (InstanceSaveMgr.h:84)
by 0xD0E2FA: WorldSession::LogoutPlayer(bool) (WorldSession.cpp:527)
by 0xC66D34: WorldSession::HandleLogoutRequestOpcode(WorldPacket&) (MiscHandler.cpp:403)
by 0xD0EA82: WorldSession::Update(unsigned int, PacketFilter&) (WorldSession.cpp:312)
by 0xD9AD66: World::UpdateSessions(unsigned int) (World.cpp:2615)
by 0xD9BEC4: World::Update(unsigned int) (World.cpp:1978)
by 0xA035E5: WorldRunnable::run() (WorldRunnable.cpp:60)
by 0xEC8D39: ACE_Based::Thread::ThreadTask(void*) (Threading.cpp:183)
by 0x55D7555: ACE_OS_Thread_Adapter::invoke() (OS_Thread_Adapter.cpp:103)
m_MonthlyQuestChanged was initialized only when loading a Player from DB and left uninitialized when creating a new Player.
Valgrind log:
Conditional jump or move depends on uninitialised value(s)
at 0x1148E2A: Player::_SaveMonthlyQuestStatus(Trinity::AutoPtr<Transaction, ACE_Thread_Mutex>&) (Player.cpp:19694)
by 0x1146510: Player::SaveToDB(bool) (Player.cpp:19191)
by 0x14F5D5C: WorldSession::HandleCharCreateCallback(Trinity::AutoPtr<PreparedResultSet, ACE_Thread_Mutex>, CharacterCreateInfo*) (CharacterHandler.cpp:660)
Closes#10620
Signed-off-by: Nay <[email protected]>
Increment the reference count of m_task in Thread::start() before spawning the actual Thread that will execute the task, otherwise the thread might finish, decRef the task and delete it.
Valgrind log of the issue:
Invalid read of size 8
at 0x1314CAD: ACE_Atomic_Op_GCC<long>::operator++() (Atomic_Op_GCC_T.inl:34)
by 0x15933FB: ACE_Based::Runnable::incReference() (Threading.h:36)
by 0x1592D2D: ACE_Based::Thread::start() (Threading.cpp:136)
by 0x1592C37: ACE_Based::Thread::Thread(ACE_Based::Runnable*) (Threading.cpp:111)
by 0xF6C463: Master::Run() (Master.cpp:195)
by 0xF725D0: main (Main.cpp:142)
Address 0x26137278 is 8 bytes inside a block of size 24 free'd
at 0x4C2B59C: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0xF67FDB: RARunnable::~RARunnable() (RARunnable.cpp:55)
by 0x1593441: ACE_Based::Runnable::decReference() (Threading.h:40)
by 0x1592E92: ACE_Based::Thread::ThreadTask(void*) (Threading.cpp:186)
by 0x515EA35: ACE_OS_Thread_Adapter::invoke() (in /usr/lib/libACE-6.0.3.so)
by 0x5F19F8D: start_thread (pthread_create.c:311)
by 0x6A46E1C: clone (clone.S:113)
Closes#10619
Initialized UpdateMask::_bits to NULL in all constructors.
UpdateMask(UpdateMask const& right) constructor sets the field count with SetCount() method before any field initialization. This means that SetCount() will call delete[] on the uninitialized _bits pointer field, leading to undefined behavior.
Assure that BIHWrap::remove(const T& obj) actually removes every reference of obj from BIHWrap.
Also add sanity checks to MDLCallback () functions to avoid array overruns.
Closes#5218
Signed-off-by: Nay <[email protected]>
jackpoz