> On Sept. 26, 2019, 5:59 p.m., Benjamin Mahler wrote: > > 3rdparty/stout/include/stout/posix/dynamiclibrary.hpp > > Lines 70-74 (original), 68-72 (patched) > > <https://reviews.apache.org/r/71388/diff/2/?file=2166909#file2166909line75> > > > > hm.. should maybe release / reset to nullptr if dlclose fails as well? > > > > E.g. if some other part of the program closes it (bug), I guess whether > > we reset here will just mean whether we issue an additional bad close call > > in the destructor. > > > > Didn't mark as an issue since the error case here already means > > something is badly broken.
I think if the close fails it is really hard to recover in some general way here. If we did that we'd either implement enough intelligence to understand all `dlclose` error scenarios (since `dlclose` does not define its own errors it looks like that would involve understanding error messages?). I think surfacing the error to the caller instead is a better idea. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71388/#review217951 ----------------------------------------------------------- On Sept. 25, 2019, 11:23 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71388/ > ----------------------------------------------------------- > > (Updated Sept. 25, 2019, 11:23 p.m.) > > > Review request for mesos, Benjamin Mahler and Jan Schlicht. > > > Repository: mesos > > > Description > ------- > > This not only simplifies our implementation of `DynamicLibrary`, but > also removes the potential for accidental file descriptor leaks. > > > Diffs > ----- > > 3rdparty/stout/include/stout/posix/dynamiclibrary.hpp > 6a50592632246b06152bee177e933f65438c07ca > > > Diff: https://reviews.apache.org/r/71388/diff/2/ > > > Testing > ------- > > `make check` > > > Thanks, > > Benjamin Bannier > >
