taku910 / crfpp

CRF++: Yet Another CRF toolkit
Other
506 stars 194 forks source link

Memory leak with the java code #6

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
Create a tagger and delete it:
// 1st way to create a tagger:
Tagger tagger = new Tagger("-m someModel -v 3 -n2");
tagger.delete();

// 2nd way to create a tagger:
Model model = new Model("-m someModel -v 3 -n2");
Tagger tagger = model.createTagger();
tagger.delete();

What is the expected output? What do you see instead?
In both cases, the delete method should call the c++ code to destroy the tagger 
from memory.
It is the case for the first exemple but not the second one.

A simple way to check it is to fill an ArrayList with taggers and then remove 
all of them by calling tagger.delete:
// 1st way to create a tagger:
           // List that will contain all the taggers
        List<Tagger> taggers = new ArrayList<Tagger>();

        for (int i = 0; i < 10000; i++) {
            // create a tagger by giving the path to the model
            Tagger tagger = new Tagger("-m someModel -v 3 -n2");
            taggers.add(tagger);
        }

        // remove all objects from TaggerVector
        for (Tagger currTagger : taggers) {
            currTagger.delete();
        }

// 2nd way to create a tagger:
            // List that will contain all the taggers
        List<Tagger> taggers = new ArrayList<Tagger>();

        // The model used to create taggers
        Model model = new Model("-m someModel -v 3 -n2");
        for (int i = 0; i < 10000; i++) {
            // create a tagger from the model created above
            Tagger tagger = model.createTagger();
            taggers.add(tagger);
        }

        // remove all objects from TaggerVector
        for (Tagger currTagger : taggers) {
            currTagger.delete();
        }

Valgring(tool to check memory leak) says that the first exemple has no memory 
leak but the second one generates memory leak (counted in mega bytes).

What version of the product are you using? On what operating system?
The latest version of crfpp: 0.57. Tested on OS X 10.8 and linux centos.

Please provide any additional information below.

The problem comes from the generated java code. In the class Model.java, the 
method createTagger calls the constuctor of Tagger.java: Tagger(cPtr, false). 
The last argument is cMemoryOwn. In our case, cMemoryOwn should be true and not 
false to say that the memory will be managed on java side and not c++ side. 
Indeed, we can notice that the delete method of Tagger.java checks that the 
flag value is true to delete (else it does nothing): if (swigCMemOwn).
In fact SWIG makes a mistake while generating the java code of model. instead 
of generating:
  public Tagger createTagger() {
    long cPtr = CRFPPJNI.Model_createTagger(swigCPtr, this);
    return (cPtr == 0) ? null : new Tagger(cPtr, false);
  }
it should generate:
  public Tagger createTagger() {
    long cPtr = CRFPPJNI.Model_createTagger(swigCPtr, this);
    return (cPtr == 0) ? null : new Tagger(cPtr, true);
  }

To sum up, the call to the tagger constructor in createTagger should be new 
Tagger(cPtr, true) instead of new Tagger(cPtr, false).

Original issue reported on code.google.com by damien.r...@gmail.com on 12 Oct 2012 at 3:54

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
This is simply a matter of adding

    %newobject CRFPP::Model::createTagger;

to CRFPP.i.

Original comment by tavianator@gmail.com on 16 May 2013 at 9:19

GoogleCodeExporter commented 9 years ago
Bump.  Here's a patch.

diff --git a/CRF++-0.58/swig/CRFPP.i b/CRF++-0.58/swig/CRFPP.i
index 212a613..cfd6dc5 100644
--- a/CRF++-0.58/swig/CRFPP.i
+++ b/CRF++-0.58/swig/CRFPP.i
@@ -5,6 +5,7 @@
 %}

 %newobject surface;
+%newobject CRFPP::Model::createTagger;

 %exception {
   try { $action }

Original comment by tavianator@gmail.com on 7 Oct 2013 at 11:21