KlausVigo / phangorn

Phylogenetic analysis in R
http://klausvigo.github.io/phangorn/
203 stars 38 forks source link

Memory leak in C_sprdist #92

Closed ms609 closed 4 years ago

ms609 commented 4 years ago

I am encountering a memory leak when I use SPR.dist to compare large numbers of trees. Over time, all my RAM is consumed.

set.seed(1)
for (i in 1:1000) {
  cat('.')
  trees <- lapply(rep(40, 50), ape::rtree, br = NULL)
  # If trees are not re-ordered, then path.dist often crashes
  trees <- structure(lapply(trees, TreeTools::Postorder), class='multiPhylo')
  phangorn::SPR.dist(trees)
}

From a very brief glance at the code, I notice that split->s_split seems to be malloced, but never freed; a possible fix to this issue is proposed. But it is probably worth taking a more careful and detailed look at the code than this to check that there are not additional leaks.

coveralls commented 4 years ago

Coverage Status

Coverage increased (+0.007%) to 69.726% when pulling a1b145249a02767b73727db6700453b286832db7 on ms609:patch-1 into a969c0ed5b3b3406ea1f090550c3d757225cd104 on KlausVigo:master.

KlausVigo commented 4 years ago

Thanks @ms609 for catching and fixing this.