JDK-4868479 : Reusing JPEGImageReader to read multiple JPEG images leaks memory
  • Type: Bug
  • Component: client-libs
  • Sub-Component: javax.imageio
  • Affected Version: 1.4.2
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: linux
  • CPU: x86
  • Submitted: 2003-05-22
  • Updated: 2003-09-15
  • Resolved: 2003-09-15
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
1.4.2_18-revFixed
Description

Name: rmT116609			Date: 05/22/2003


FULL PRODUCT VERSION :
java version "1.4.2-beta"
Java(TM) 2 Runtime Environment, Standard Edition (build 1.4.2-beta-b19)
Java HotSpot(TM) Client VM (build 1.4.2-beta-b19, mixed mode)

FULL OS VERSION :
Linux xxx.xxx.xxx.xxx 2.4.20-13.9 #1 Mon May 12 10:55:37 EDT 2003 i686 i686 i386 GNU/Linux

A DESCRIPTION OF THE PROBLEM :
Reusing the same JPEGImageReader to read multiple JPEG images leeks memory.

NOTE: This is not related to the bug that JPEGImageReader leeks memory if    dispose() is not called after finishing with the JPEGImageReader. If an  JPEGImageReader is reused if leeks memory for good and the memory cannot be reclaimed with subsequent call to dispose().

 Since the leek is very small (<1k per image) it will not be noticed by normal  client code but with code that is meant to process millions of images over a long period of time, it will be noticable.

 The memory leek occures in the native part of the JPEGImageReader where the reference to the object is lost.

 STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
 1. Create a JPEGImageReader
 2. Read multiple (>100000) small images with the JPEGImageReader, do not call  reset() in between reads
 3. Run the code with -runhprof switch

 EXPECTED VERSUS ACTUAL BEHAVIOR :
 EXPECTED -
 No memory should be leeked
 ACTUAL -
 Each time the JPEGImageReader is reused a small byte buffer (size depends on  the image size) is leeked.

 Here is a snipet from the java.hprof.txt SITES section showing the memoryleek

 SITES BEGIN (ordered by live bytes) Sun May 18 15:12:47 2003
          percent         live       alloc'ed  stack class
 rank   self  accum    bytes objs   bytes objs trace name
    6  3.02% 80.41%    51744   66   51744   66   840 [B

 You can tell it is a memory leek because non of the 66 objects (there are 66 iterations in the attached test code) is never freed.

 and the associated stack trace:
 TRACE 840:
 java.awt.image.DataBufferByte.<init>(DataBufferByte.java:42)       com.sun.imageio.plugins.jpeg.JPEGImageReader.readInternal(JPEGImageReader.java:963)
com.sun.imageio.plugins.jpeg.JPEGImageReader.read(JPEGImageReader.java:853)
javax.imageio.ImageReader.read(ImageReader.java:919)


 REPRODUCIBILITY :
 This bug can be reproduced always.

---------- BEGIN SOURCE ----------
import javax.imageio.*;
import javax.imageio.stream.*;
import java.util.*;
import java.io.*;

public class JPEGImageReaderTest {

  public static void main(String[] args) {
    if (args.length != 1) {
      System.err.println("USAGE: java JPEGImageReaderTest <jpegfile>");
      System.exit(1);
    }

    try {
      ImageReader reader =
	(ImageReader) ImageIO.getImageReadersByFormatName("jpeg").next();
    
      long start = System.currentTimeMillis();
      for (int i = 0; i < 66; i++) {
	System.out.print('.');
	reader.setInput(new FileImageInputStream(new File(args[0])));
	reader.read(0);
      }
      reader.dispose();
      System.out.println("\ntook " + (System.currentTimeMillis() - start) +
			 " ms");
      System.gc(); System.gc();
    } catch (Exception e) {
      e.printStackTrace();
    }
  }
}
---------- END SOURCE ----------

CUSTOMER SUBMITTED WORKAROUND :
1. Do not reuse JPEGImageReader - this option means that you have to instantiate a  new JPEGImageReader for every image... slow

2. call reset(), but since reset() has a call to System.gc() in it, it is very slow.
(Review ID: 186036) 
======================================================================

Comments
CONVERTED DATA BugTraq+ Release Management Values COMMIT TO FIX: tiger FIXED IN: tiger INTEGRATED IN: tiger tiger-b20
2004-06-14

EVALUATION There are two issues here. One involves the documentation for the ImageReader.reset() method. The spec for this method is rather vague, and should be updated to say that it is important for developers to call reset() after each iteration if they are reusing the same ImageReader to read multiple images. The second issue is that we seem to be leaking resources in the JPEGImageReader (and presumably the JPEGImageWriter as well) if the developer does not explicitly call reset() between successive reads. This is due to the fact that we create a new global JNI reference to the temporary pixel buffer without ever deleting that ref when we're done with it. This means we keep a reference to a DataBuffer (as the submitter mentioned) that never gets garbage collected. We can fix this by checking for a non-null pixel buffer object in setPixelBuffer() (imageioJPEG.c, line 263), and if it is non-null, delete the existing global ref before we create a new one. Perhaps we could simply call resetPixelBuffer() as the first step in setPixelBuffer(). (The submitter should be aware that it is important to call reset() if you are reusing an ImageReader/Writer. I understand that the submitter is avoiding this call because it is invoking System.gc(), but we are working to remove the explicit gc() calls as part of the fixes for 4867874 and 4827358.) ###@###.### 2003-05-23 The above evaluation was a little premature on my part. After looking over the spec a bit more, it becomes clear that a call to setInput/Output() should reset any internal state so as to avoid leaking native resources. So the attached testcase should work as written (without an explicit reset() call between each iteration. Sorry for the confusion. The fix suggested above would work, but a more complete solution would be to reset the appropriate internal state when setInput/Output() is called. In other words, the reset() and setInput/Output() methods should just share a method that resets internal resources. This will avoid leaking the DataBuffer objects mentioned earlier. This work would best be completed alongside the fixes for 4867874 and 4827358. ###@###.### 2003-08-26
2003-08-26