> On Dec. 13, 2016, 11:13 a.m., Benjamin Bannier wrote: > > src/linux/ldd.cpp, lines 82-83 > > <https://reviews.apache.org/r/53791/diff/6/?file=1571593#file1571593line82> > > > > Let's move this up right after the check `needed.contains(path)`. > > > > Right now in pathological cases like e.g., a`libA` depending on `libB`, > > and `libB` depending on `libA`, we would recurse indefinitely. > > James Peach wrote: > Adding the path to the `needed` set means that we need the path and have > already calculated the dependencies for that path. If we add a path before > having all its dependencies, we would return early in the > `needed.contains(path)` check.
An entry in `needed` is the only indication on whether we have already visited a node in the dependency graph (which is not necessarily a tree). If you do not add `path` to `needed` before recursing into `collectDependencies`, `path` might be visited again as a dependency in pathological cases involving cyclic shared library dependencies (which the loader might be fine with). In such a case this code would recurse indefinitely. It looks like a fix might be to add `path` to `needed` after you have found that it currently is not in `needed`, but before you recurse. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53791/#review158971 ----------------------------------------------------------- On Dec. 13, 2016, 7:34 p.m., James Peach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53791/ > ----------------------------------------------------------- > > (Updated Dec. 13, 2016, 7:34 p.m.) > > > Review request for mesos, Gilbert Song, Jie Yu, Kevin Klues, and Jiang Yan Xu. > > > Bugs: MESOS-6588 > https://issues.apache.org/jira/browse/MESOS-6588 > > > Repository: mesos > > > Description > ------- > > The dependencies for the programs we need in the Linux root > filesystem vary over time and across distributions. Since Stout > has support for parsing the library dependencies of ELF binaries, > use this to collect the necessary dependencies when constructing > a root filesystem for the tests. > > > Diffs > ----- > > src/Makefile.am a4c03c2b918816e6dd8872d37e5208f055619c47 > src/tests/containerizer/rootfs.hpp 6bc3835cbb62536ec933ef38c9e15138b8611e5f > src/tests/containerizer/rootfs.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/53791/diff/ > > > Testing > ------- > > sudo make check (Fedora 24) > > > Thanks, > > James Peach > >
