Opened 6 years ago

Closed 5 years ago

#12622 closed defect (duplicate)

cliquer memory leaks

Reported by: jason Owned by: jason, ncohen, rlm
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: graph theory Keywords:
Cc: ncohen Merged in:
Authors: Jason Grout Reviewers: David Coudert, Nathann Cohen
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:


See for the initial report. Basically, Sage's interface to cliquer has a number of memory leaks:

  1. in cliquer.pyx, graph_new is called several times without a corresponding graph_free.
  1. The lists that are returned from the cliquer functions are not deallocated.
  1. Inside the cliquer spkg, the cl.c file allocates lots of option structs through calls to sage_init_clique_opt(), but never deallocates them.

Attachments (1)

trac-12622-cliquer-leaks.patch (1.2 KB) - added by jason 6 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 6 years ago by jason

  • Status changed from new to needs_review

Here are my changes to the cl.c file inside the cliquer spkg. Do you happen to have time to package these up, Nathann? I was a little confused since usually unmodified source is supposed to be in the src/ directory, but it looks like we have sage-specific stuff in the src/ directory in the cl.c file.

diff -Naur cliquer-1.2.p10/src/cl.c cliquer-1.2.p11/src/cl.c
--- cliquer-1.2.p10/src/cl.c	2010-02-15 22:26:56.000000000 -0600
+++ cliquer-1.2.p11/src/cl.c	2012-03-03 03:11:03.000000000 -0600
@@ -97,9 +97,11 @@
   set_t s;
   int i,l;
+  clique_options *opts = sage_init_clique_opt();
-				  sage_init_clique_opt());
+                                  opts);
+  free(opts);
   // Writing the answer into a int [] to be read by Sage
   int size=set_size(s);
@@ -132,9 +134,10 @@
   int i,j,l;
+  clique_options *opts = sage_init_clique_opt();
-			     maximal,sage_init_clique_opt());
+			     maximal,opts);
+  free(opts);
   int size=set_size(clique_list[0]);
@@ -162,7 +165,9 @@
   clique_options *opts;
-  return clique_unweighted_max_weight(g,opts);
+  int result = clique_unweighted_max_weight(g,opts);
+  free(opts);
+  return result;

comment:2 Changed 6 years ago by jason

  • Status changed from needs_review to needs_work

Changed 6 years ago by jason

comment:3 Changed 6 years ago by jason

I'm not so sure about using sage_free to free memory allocated by cliquer's malloc. Could that cause problems?

Anyways, the attached patch and the above changes make a *huge* difference in memory usage when calculating cliques over a large range of graphs.

comment:4 Changed 6 years ago by dcoudert

  • Reviewers set to David Coudert

Apparently this patch is ready to be reviewed, but has still a needs_work status. I tried it and it is working correctly (install, tests, docbuild,...).

However, I don't know how to evaluate the memory improvements. Could you provide an example ?

comment:5 Changed 5 years ago by ncohen

  • Milestone changed from sage-5.9 to sage-duplicate/invalid/wontfix
  • Reviewers changed from David Coudert to David Coudert, Nathann Cohen
  • Status changed from needs_work to positive_review

Duplicate of #12905


comment:6 Changed 5 years ago by jdemeyer

  • Resolution set to duplicate
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.