mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <benjamin.bann...@mesosphere.io>
Subject Re: Review Request 46094: Fixed memory leaks in Encoder/Decoder tests in libprocess.
Date Tue, 12 Apr 2016 14:16:19 GMT

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




3rdparty/libprocess/src/tests/decoder_tests.cpp (line 202)
<https://reviews.apache.org/r/46094/#comment191828>

    Here and everywhere below: This still leaks if any of above `EXPECT_*` fail; in that case
we continue running tests.
    
    Since we `ASSERT` that we only got a single response we might as well do just
    
        Owned<http::Response> reponse = response[0];
        
    If the goal of this nice effort is to be able to run tests under leak checkers (and potentially
correlate failures to memory problems), I'd really appreciate using some smart pointer for
automatic cleanup instead.


- Benjamin Bannier


On April 12, 2016, 4:02 p.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46094/
> -----------------------------------------------------------
> 
> (Updated April 12, 2016, 4:02 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Fixed memory leaks in Encoder/Decoder tests in libprocess.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/tests/decoder_tests.cpp bd990c5eb77e47d7f617199bcc89e9432af7cc51

>   3rdparty/libprocess/src/tests/encoder_tests.cpp 61ec8d8722245179a929e954e6338606973b819b

> 
> Diff: https://reviews.apache.org/r/46094/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> Verified that the number of leaked allocations decreases (from ~129 to ~92) with this
change. Obviously there are more leaks to investigate but at first glance they seem a bit
more subtle.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>


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