Bug 29263 - Memory leak in VMDouble.toString()
Summary: Memory leak in VMDouble.toString()
Status: WAITING
Alias: None
Product: classpath
Classification: Unclassified
Component: classpath (show other bugs)
Version: 0.92
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-09-28 07:24 UTC by freebeans
Modified: 2012-02-22 12:59 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
Patch (320 bytes, patch)
2007-06-13 16:12 UTC, Szilveszter Ordog
Details | Diff
also free any _p5s allocated during the dtoa (428 bytes, patch)
2009-07-10 19:39 UTC, Ben Gardiner
Details | Diff
Additional patch for VMDouble.parseDouble() (571 bytes, patch)
2010-01-27 07:15 UTC, Roland Brand
Details | Diff
correct the CNI version of VMDouble::parseDouble() memory leak (484 bytes, patch)
2011-01-14 11:26 UTC, Charles G.
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description freebeans 2006-09-28 07:24:36 UTC
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.
Comment 1 Tom Tromey 2006-09-28 17:27:33 UTC
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?
Comment 2 freebeans 2006-09-28 23:22:53 UTC
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.
Comment 3 Szilveszter Ordog 2007-06-13 16:12:57 UTC
Created attachment 13698 [details]
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).
Comment 4 freebeans 2008-02-15 16:25:55 UTC
Comment #3 patch works fine.
(I'm sorry that I posted incorrect solution)
Comment 5 Ben Gardiner 2009-07-10 19:39:24 UTC
Created attachment 18177 [details]
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.
Comment 6 Erik J Groeneveld 2009-07-29 14:01:21 UTC
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?
Comment 7 Roland Brand 2010-01-27 07:15:44 UTC
Created attachment 19722 [details]
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.
Comment 8 Erik J Groeneveld 2010-01-27 11:00:19 UTC
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 #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.
>
Comment 9 Charles G. 2011-01-14 11:26:30 UTC
Created attachment 22964 [details]
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
Comment 10 xiaoyuanbo 2012-02-22 12:59:01 UTC
you ask me memory hole