http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/updater/updater.cpp#3607 free is never called on mbuf

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ded40585e08f&exclusion_profile=false http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/rstrong@mozilla.com-ded40585e08f/try-linux64-asan/try_ubuntu64-asan_vm_test-mochitest-other-bm117-tests1-linux64-build2.txt.gz 19:56:20 INFO - ==5817==ERROR: LeakSanitizer: detected memory leaks 19:56:20 INFO - Direct leak of 145 byte(s) in 1 object(s) allocated from: 19:56:20 INFO - #0 0x471501 in __interceptor_malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:74 19:56:20 INFO - #1 0x49595f in GetManifestContents(char const*) /builds/slave/try-l64-asan-00000000000000000/build/src/toolkit/mozapps/update/updater/updater.cpp:3633 19:56:20 INFO - #2 0x4920c1 in DoUpdate /builds/slave/try-l64-asan-00000000000000000/build/src/toolkit/mozapps/update/updater/updater.cpp:3750 19:56:20 INFO - #3 0x4920c1 in UpdateThreadFunc(void*) /builds/slave/try-l64-asan-00000000000000000/build/src/toolkit/mozapps/update/updater/updater.cpp:2278 19:56:20 INFO - #4 0x7f4c7e8a7e99 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x7e99)

I just independently rediscovered this leak, so I'll add some detail for anybody that might end up working on it. The leaked object is the buffer returned from GetManifestContents in updater.cpp, whichever buffer that is for the specific platform (wrb on Windows, mbuf/rb elsewhere). That function is called in two places: For the call in DoUpdate, just freeing the buffer normally at the end of the function is all that's needed. I would use a mozilla::UniquePtr to make that easy. For the call in AddPreCompleteActions, freeing the buffer at the end of that function would break things right now. The problem is that pointers into the middle of the buffer are saved by the various Action types in their Parse methods, and those pointers are used after AddPreCompleteActions returns, when DoUpdate prepares and executes the whole ActionList. Either the buffer needs to be somehow preserved until the end of DoUpdate and then freed (by having a reference to it somewhere in DoUpdate's scope, or using reference counting where each Action that's created increments the count, or something), or the Action types need to make their own copies of the parts of the buffer that they care about, so that they no longer depend on the whole thing sticking around. This is not a high-priority leak because a) it amounts to a handful of kilobytes, b) it's not in the main application, it's in the updater binary, which is only invoked during application updates, and c) the updater exits soon after these buffers would be freed.

Pushed by rstrong@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4fe736c38366 Fix leaks in updater.cpp and nsUpdateDriver.cpp. r=spohl