> On Nov. 16, 2016, 8:05 p.m., Joseph Wu wrote: > > 3rdparty/libprocess/src/process.cpp, lines 1221-1222 > > <https://reviews.apache.org/r/53817/diff/1/?file=1565267#file1565267line1221> > > > > It does look like this is a leak, but due to joining and recreating the > > thread pool between reinitializations, the `__executor__` will be recreated. > > > > Deleting a thread local variable like this will only delete one > > instance of it, whereas there could be one instance per thread. We should > > consider changing the `_executor_` to an `Owned<>` value.
Per our discussion, the `__executor__` in the main thread is not getting recreated, leading to the failure of `ProcessTest.Defer3` when run in repetition. We can add a `delete` to `process::finalize()` to solve this. We should also add a `delete` statement into the worker threads themselves to prevent the `_executor_` pointers from leaking across reinitializations. - Greg ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53817/#review156111 ----------------------------------------------------------- On Nov. 16, 2016, 7:07 p.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53817/ > ----------------------------------------------------------- > > (Updated Nov. 16, 2016, 7:07 p.m.) > > > Review request for mesos, Benjamin Mahler, Joseph Wu, and Vinod Kone. > > > Bugs: MESOS-5966 > https://issues.apache.org/jira/browse/MESOS-5966 > > > Repository: mesos > > > Description > ------- > > The `finalize()` function in libprocess previously did > not delete the thread-local `Executor` which is used to > `defer` execution to an unspecified context. This object > must be deleted during finalization so that the > `__executor__` macro creates a new process if libprocess > is subsequently reinitialized. > > > Diffs > ----- > > 3rdparty/libprocess/src/process.cpp > ab2b5a9d38a3001d6a5daa1807fecb630c4b154d > > Diff: https://reviews.apache.org/r/53817/diff/ > > > Testing > ------- > > Testing details are found at the end of this review chain. > > > Thanks, > > Greg Mann > >
