Page MenuHomeWildfire Games

Fix glooxwrapper Message::when() leak in rP16507
ClosedPublic

Authored by elexis on Sep 14 2019, 6:49 PM.

Details

Summary

This fixes a memory leak in rP16507. Also puts one member on the stack that doesnt have to be a pointer.

Test Plan

For example run with valgrind, enter the lobby, possibly enable the --track-origin option.
Without the patch you get this upon closing the app:

==2781742== 160 bytes in 20 blocks are definitely lost in loss record 2,279 of 3,300
==2781742==    at 0x4838DEF: operator new(unsigned long) (vg_replace_malloc.c:334)
==2781742==    by 0x65F171: glooxwrapper::Message::when() const (glooxwrapper.cpp:578)
==2781742==    by 0x651CB9: XmppClient::ComputeTimestamp(glooxwrapper::Message const&) (XmppClient.cpp:1105)
==2781742==    by 0x651B52: XmppClient::handleMUCMessage(glooxwrapper::MUCRoom&, glooxwrapper::Message const&, bool) (XmppClient.cpp:753)
==2781742==    by 0x65207D: non-virtual thunk to XmppClient::handleMUCMessage(glooxwrapper::MUCRoom&, glooxwrapper::Message const&, bool) (XmppClient.cpp:0)
==2781742==    by 0x666219: glooxwrapper::MUCRoomHandlerWrapper::handleMUCMessage(gloox::MUCRoom*, gloox::Message const&, bool) (glooxwrapper.cpp:157)
==2781742==    by 0x605D1C7: gloox::MUCRoom::handleMessage(gloox::Message const&, gloox::MessageSession*) (in /usr/lib/libgloox.so.17.0.0)
==2781742==    by 0x6011221: gloox::ClientBase::notifyMessageHandlers(gloox::Message&) (in /usr/lib/libgloox.so.17.0.0)
==2781742==    by 0x6015E47: gloox::ClientBase::handleTag(gloox::Tag*) (in /usr/lib/libgloox.so.17.0.0)
==2781742==    by 0x600B552: gloox::Parser::closeTag() (in /usr/lib/libgloox.so.17.0.0)
==2781742==    by 0x600CCA0: gloox::Parser::feed(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) (in /usr/lib/libgloox.so.17.0.0)
==2781742==    by 0x6010B5A: gloox::ClientBase::parse(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (in /usr/lib/libgloox.so.17.0.0)
==2781742== 
==2781742== 320 bytes in 40 blocks are definitely lost in loss record 2,697 of 3,300
==2781742==    at 0x4838DEF: operator new(unsigned long) (vg_replace_malloc.c:334)
==2781742==    by 0x65F171: glooxwrapper::Message::when() const (glooxwrapper.cpp:578)
==2781742==    by 0x651DE2: XmppClient::ComputeTimestamp(glooxwrapper::Message const&) (XmppClient.cpp:1111)
==2781742==    by 0x651B52: XmppClient::handleMUCMessage(glooxwrapper::MUCRoom&, glooxwrapper::Message const&, bool) (XmppClient.cpp:753)
==2781742==    by 0x65207D: non-virtual thunk to XmppClient::handleMUCMessage(glooxwrapper::MUCRoom&, glooxwrapper::Message const&, bool) (XmppClient.cpp:0)
==2781742==    by 0x666219: glooxwrapper::MUCRoomHandlerWrapper::handleMUCMessage(gloox::MUCRoom*, gloox::Message const&, bool) (glooxwrapper.cpp:157)
==2781742==    by 0x605D1C7: gloox::MUCRoom::handleMessage(gloox::Message const&, gloox::MessageSession*) (in /usr/lib/libgloox.so.17.0.0)
==2781742==    by 0x6011221: gloox::ClientBase::notifyMessageHandlers(gloox::Message&) (in /usr/lib/libgloox.so.17.0.0)
==2781742==    by 0x6015E47: gloox::ClientBase::handleTag(gloox::Tag*) (in /usr/lib/libgloox.so.17.0.0)
==2781742==    by 0x600B552: gloox::Parser::closeTag() (in /usr/lib/libgloox.so.17.0.0)
==2781742==    by 0x600CCA0: gloox::Parser::feed(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) (in /usr/lib/libgloox.so.17.0.0)
==2781742==    by 0x6010B5A: gloox::ClientBase::parse(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (in /usr/lib/libgloox.so.17.0.0)

Notice it's gone after the patch.
Notice the DelayedDelivery class has purpose (instead of only returning the gloox timestamp string), for example it contains also some reason() field:
https://camaya.net/api/gloox-1.0.22/classgloox_1_1DelayedDelivery.html#ab4a59093f61a37c5d4cd3a065d2b1a9f

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

elexis created this revision.Sep 14 2019, 6:49 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/686/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/171/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/lobby/glooxwrapper/glooxwrapper.h
|  88| namespace·glooxwrapper
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceglooxwrapper{' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/694/display/redirect

This revision was not accepted when it landed; it landed in state Needs Review.Sep 15 2019, 12:28 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.