Re: [1/6] mesos git commit: Fixed a memory leak in the scheduler driver.

On Wed, Mar 30, 2016 at 2:17 PM, Neil Conway wrote: > On Wed, Mar 30, 2016 at 4:57 PM, Benjamin Mahler > wrote: > > Yikes! (3) being not true to me means that I needed non-local reasoning > to > > determine the optionality. > > Sorry: to clarify, I

Re: [1/6] mesos git commit: Fixed a memory leak in the scheduler driver.

On Wed, Mar 30, 2016 at 4:57 PM, Benjamin Mahler wrote: > Yikes! (3) being not true to me means that I needed non-local reasoning to > determine the optionality. Sorry: to clarify, I didn't mean "there is not always a latch" in the code in question. I meant: "writing 'delete

Re: [1/6] mesos git commit: Fixed a memory leak in the scheduler driver.

Yikes! (3) being not true to me means that I needed non-local reasoning to determine the optionality. I reason about symmetric operations like new/delete in a similar way to other symmetric operations like open/close. Even though close(-1) returns EBADF and doesn't do something bad, it's

Re: [1/6] mesos git commit: Fixed a memory leak in the scheduler driver.

(3) is not true, though: as written, there may or may not be a latch (the code works correctly correct either way). Using an operation (delete) that works fine for null pointers as a way to imply that a pointer is NOT null does not seem like the best arrangement. As you say, there should be

Re: [1/6] mesos git commit: Fixed a memory leak in the scheduler driver.

I'm not sure the null check was in place for the safety of deletion so much as to make the code easier to reason about: if (process != NULL) { terminate(process); wait(process); delete process; } if (credential != NULL) { delete credential; } delete latch; Here, (1)

Re: [1/6] mesos git commit: Fixed a memory leak in the scheduler driver.

+1 Refer to this doc for the detail of deleting null: http://www.cplusplus.com/reference/new/operator%20delete/ Thanks Klaus > On Mar 30, 2016, at 07:24, Neil Conway wrote: > > On Tue, Mar 29, 2016 at 7:19

Re: [1/6] mesos git commit: Fixed a memory leak in the scheduler driver.

On Tue, Mar 29, 2016 at 7:19 PM, wrote: > --- a/src/sched/sched.cpp > +++ b/src/sched/sched.cpp > @@ -1808,6 +1808,10 @@ MesosSchedulerDriver::~MesosSchedulerDriver() > delete process; >} > > + if (credential != NULL) { > +delete credential; > + } `delete`