mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Neil Conway <neil.con...@gmail.com>
Subject Re: Review Request 53387: Avoided memory leaks in exception/error paths in tests.
Date Thu, 03 Nov 2016 16:23:02 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53387/
-----------------------------------------------------------

(Updated Nov. 3, 2016, 4:23 p.m.)


Review request for mesos and Joseph Wu.


Changes
-------

Address review comments; fix some more leaks in other tests.


Summary (updated)
-----------------

Avoided memory leaks in exception/error paths in tests.


Repository: mesos


Description (updated)
-------

`clang-tidy` rightly points out that certain `ASSERT` failures can
result in leaking some heap-allocated values that aren't wrapped in a
smart pointer. This commit fixes the cases that `clang-tidy` complains
about by wrapping the values in `Owned<T>`.

Note that there are many other places in the tests that leak resources
if an exception occurs. The proper fix is usually to use a smart pointer
rather than a raw pointer; several of these APIs should be changed to
return Owned<T>, for example. However, this is not always easy/clean, in
part because of limitations of the current `Owned<T>` and `Shared<T>`
types (e.g., MESOS-5122, MESOS-6496). So for now, just fix the cases
that clang-tidy complains about and a few other similar instances.


Diffs (updated)
-----

  src/tests/containerizer/docker_volume_isolator_tests.cpp ca7bffd3b1773a11a4679d114885d3edd977b02b

  src/tests/containerizer/mesos_containerizer_tests.cpp 4df537747d84daa68c29e2d05b22fa386a4a16db


Diff: https://reviews.apache.org/r/53387/diff/


Testing
-------

`make check`

Verified that observed `clang-tidy` warnings go away with this change.


Thanks,

Neil Conway


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message