Memory leak in VMDouble.toString()

[ingebot]

Note: this issue was migrated automatically using bugzilla2github

Original bug ID: BZ#29263 From: freebeans@xqb.biglobe.ne.jp Reported version: 0.92 CC: bug-classpath@gnu.org, erik@cq2.nl, roland.brand@ergon.ch, tromey@gcc.gnu.org, xiaoyuanbo@yeah.net

[ingebot]

Comment author: freebeans@xqb.biglobe.ne.jp

VMDouble.toString() calls _dtoa() function. _dtoa() calls _dtoa_r() function.

--- dtoa.c line 904 ---
  p = _dtoa_r (&reent, _d, mode, ndigits, decpt, sign, rve, float_type);
  strcpy (buf, p);

and _dtoa_r() allocate memory for result string.

--- dtoa.c line 446 ---
  ptr->_result = Balloc (ptr, ptr->_result_k);
  s = s0 = (char *) ptr->_result;
  ...
  return s0;

But _dtoa() does not free that memory.

[ingebot]

Comment author: Tom Tromey <tromey@gcc.gnu.org>

I would have thought that the loop in _dtoa that cleans up the freelist would have freed this. Does that not work?

Hmm, maybe it is because Balloc doesn't put the new object into the freelist?

Can you look at this?

[ingebot]

Comment author: freebeans@xqb.biglobe.ne.jp

I think _dtoa() does not clean up freelist, and result buffer.

I modified following points in _dtoa():

1. free "result" memory (variable "p")
2. changed clean up loop

  p = _dtoa_r (&reent, _d, mode, ndigits, decpt, sign, rve, float_type);
  strcpy (buf, p);
// 2006/09/26 freebeans START
  free(p);

//  for (i = 0; i < reent._result_k; ++i)
// 2006/09/26 freebeans END
  for (i = 0; i < reent._max_k; ++i)
    {

It seems the problem has been fixed.

[ingebot]

Comment author: Szilveszter Ordog <slipszi@gmail.com>

Created attachment 13698 Patch

The solution from comment 2 is not entirely correct since _dtoa_r sometimes returns a static string (Infinity, NaN, 0). However, we can free reent._result (if it isn't NULL).

Attached file: classpath-vmdouble-tostring.patch (text/plain, 320 bytes) Description: Patch

[ingebot]

Comment author: freebeans@xqb.biglobe.ne.jp

Comment 3 patch works fine. (I'm sorry that I posted incorrect solution)

[ingebot]

Comment author: Ben Gardiner <BenGardiner@nanometrics.ca>

Created attachment 18177 also free any _p5s allocated during the dtoa

I tried the patch in comment #3 but found that there were still some leaks with the following test program, using the mtrace feature of glibc:

import java.util.Random;

public class DToATester
{

  /**
   * @ param args
   */
  public static void main(String[] args)
  {
    Random random = new Random(1);
    while(true)
    {
      double val = Double.longBitsToDouble(random.nextLong());
      System.out.print(val + " ");
    }
  }
}

turns out that there are some additional _Jv_Bigint's allocated in pow5mult assigned to the '_p5s' linked-list member of reent. The following patch gets rid of all allocations made in mprec_calloc -- according the mtrace. It was created against gcj-4.3.3's import of classpath which I think is 0.92 according to the classpath README.

Attached file: dtoa.patch (text/plain, 428 bytes) Description: also free any _p5s allocated during the dtoa

[ingebot]

Comment author: Erik J Groeneveld <erik@cq2.nl>

I found exactly this bug and created a patch for it, not knowing someone else did the same at the same time.

The bug causes GB's of memory leaks when indexing with Lucene (via GCJ).

I am very pleased that this patch is reported and I would like to vote for it to be applied soon.

At what time can we see this patch being applied?

[ingebot]

Comment author: Roland Brand <roland.brand@ergon.ch>

Created attachment 19722 Additional patch for VMDouble.parseDouble()

I stumbled over the same leaks in version 0.98 of classpath and fixed them with the patches in this bug report.

The test in comment 5 unveiled another memory leak in VMDouble.parseDouble() which is called by VMDouble.toString(). Problem and solution are of the same kind as in the dtoa.patch above.

Hope these patches will make it to the release someday.

Attached file: VMDouble.patch (text/plain, 571 bytes) Description: Additional patch for VMDouble.parseDouble()

[ingebot]

Comment author: Erik J Groeneveld <erik@cq2.nl>

Subject: Re: Memory leak in VMDouble.toString()

Thanks for adding an additional patch.

I wonder what it takes to get these patches applied?

I believe the code is bloating here and most of the reent stuff here is obsolete. Applying these patches would best be done together with some serious refactoring. To be concrete: remove the 'optimization' for allocating bigints as it is only making performance worse. But if I do it, will it be applied?

Best regards, Erik

E.J. Groeneveld Seek You Too twitter, skype: ejgroene mobiel: 0624 584 029

On Wed, Jan 27, 2010 at 08:15, roland dot brand at ergon dot ch <gcc-bugzilla@ gcc.gnu.org> wrote:

------- Comment BZ#7 from roland dot brand at ergon dot ch  2010-01-27 07:15 ------- Created an attachment (id=19722)  --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=19722&action=view) Additional patch for VMDouble.parseDouble()

I stumbled over the same leaks in version 0.98 of classpath and fixed them with the patches in this bug report.

The test in comment 5 unveiled another memory leak in VMDouble.parseDouble() which is called by VMDouble.toString(). Problem and solution are of the same kind as in the dtoa.patch above.

Hope these patches will make it to the release someday.

--

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29263

------- You are receiving this mail because: ------- You are on the CC list for the bug, or are watching someone who is.

[ingebot]

Comment author: Charles G. <charles.gaumet@gmail.com>

Created attachment 22964 correct the CNI version of VMDouble::parseDouble() memory leak

Patch from comment 7 is for JNI version of VMDouble.parseDouble() ; to correct the same bug for CNI version, I corrected in a similar way the file libjava/java/lang/natVMDouble.cc

Attached file: parseDouble.patch (text/plain, 484 bytes) Description: correct the CNI version of VMDouble::parseDouble() memory leak