Discussion:
suspendProcessing() and dispose() memory leak
Lukasz Matuszczak
2005-02-17 12:59:09 UTC
Permalink
Hello,

I am still fighting with "dispose()" problems, and I discovered something, which I consider a bug (it appears in 1.5.1 release and in a latest version from CVS head).
The problem is when I call method "suspendProcessing()" followed by "dispose()" on JSVGCanvas object,
the JSVGCanvas won't be collected by garbage collector. This is somehow connected with UpdateManager ant its RunnableQueue, I think.
You may ask why i call dispose() after suspendProcessing() (why not only dispose()). The answer is that I have my own queue of events in my application, and sometimes I get a request to deactivate the SVG screen (suspendProcessing), and soon after that to close it (close). I handle these events one after another (of course AWT in event dispatch thread).

The sample code is:
MemoryLeakTest.java:
-----------------------------------
import java.awt.BorderLayout;
import java.awt.Dimension;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.io.InputStreamReader;
import java.io.Reader;
import java.lang.ref.ReferenceQueue;
import java.lang.ref.WeakReference;
import java.net.URL;
import java.net.URLConnection;
import javax.swing.BoxLayout;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JPanel;
import org.apache.batik.dom.svg.SAXSVGDocumentFactory;
import org.apache.batik.swing.JSVGCanvas;
import org.apache.batik.util.XMLResourceDescriptor;
import org.w3c.dom.Document;
import org.w3c.dom.svg.SVGDocument;

public class MemoryLeakTest extends JFrame {
JPanel left;
JPanel right = new JPanel();
Document doc = null;
JSVGCanvas canvas = null;

public MemoryLeakTest() {
right.setLayout(new BorderLayout());
this.getContentPane().setLayout(new BorderLayout());
this.setSize(new Dimension(1024, 768));
this.setTitle("MemoryTestLeak");
JButton button1 = new JButton("Start");
JButton button2 = new JButton("Stop");
JButton button3 = new JButton("GC");
button1.addActionListener(new ActionListener() {
public void actionPerformed(ActionEvent e) {
start();
}
});
button2.addActionListener(new ActionListener() {
public void actionPerformed(ActionEvent e) {
stop();
}
});
button3.addActionListener(new ActionListener() {
public void actionPerformed(ActionEvent e) {
gc();
}
});
left = new JPanel();
left.setLayout(new BoxLayout(left, BoxLayout.Y_AXIS));
left.add(button1);
left.add(button2);
left.add(button3);
this.getContentPane().setLayout(new BorderLayout());
this.getContentPane().add(left, BorderLayout.WEST);
this.getContentPane().add(right, BorderLayout.CENTER);
start();
}

private void start() {
if (canvas == null) {
canvas = new JSVGCanvas();
final ReferenceQueue rq = new ReferenceQueue();
final WeakReference ref = new WeakReference(canvas, rq);
new Thread() {
public void run() {
try {
rq.remove();
System.out.println("CANVAS removed "+ref);
} catch (InterruptedException e) {
e.printStackTrace();
}
}
}
.start();
try {
loadDocument();
} catch (Exception e) {
e.printStackTrace();
}
right.add(canvas, BorderLayout.CENTER);
pack();
}
}

private void stop() {
if (canvas!=null) {
canvas.suspendProcessing();
canvas.dispose();
canvas = null;
doc = null;
right.removeAll();
this.invalidate();
this.pack();
System.out.println(
Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory());
}
}

private void gc() {
System.gc();
System.out.println(
Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory());
}

private void loadDocument() throws Exception {
String stringURL = "file:circle.svg";
Reader reader = null;
SVGDocument svgDoc = null;
URL url = new URL(stringURL);
URLConnection connection;
connection = url.openConnection();
reader = new InputStreamReader(connection.getInputStream());
String parser = XMLResourceDescriptor.getXMLParserClassName();
SAXSVGDocumentFactory f = new SAXSVGDocumentFactory(parser);
doc = f.createSVGDocument("file:circle.svg", reader);
canvas.setDocumentState(JSVGCanvas.ALWAYS_DYNAMIC);
canvas.setDoubleBufferedRendering(true);
canvas.setSize(new Dimension(800, 600));
canvas.setDocument(doc);
}

public static void main(String[] args) {
JFrame frame = new MemoryLeakTest();
frame.pack();
frame.setVisible(true);
}
}
-------------------------------

circle.svg:
---------------------------
<?xml version="1.0" encoding="windows-1250" standalone="no"?>
<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN"
"http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
<svg width="12cm" height="4cm" viewBox="0 0 1200 400"
xmlns="http://www.w3.org/2000/svg" version="1.1">
<desc>Example circle01 - circle filled with red and stroked with blue</desc>
<circle cx="600" cy="200" r="100"
fill="red" stroke="blue" stroke-width="10" />

</svg>
------------------------------

After pressing stop and then gc one should see "CANVAS removed ***@e2892b" message, but it appears hardly ever.

Regards,
Lukasz
Lukasz Matuszczak
2005-02-17 13:01:44 UTC
Permalink
Sorry for HTML.
Lukasz
Thomas DeWeese
2005-02-18 12:01:01 UTC
Permalink
Hi Lukasz,
Post by Lukasz Matuszczak
I am still fighting with "dispose()" problems, and I discovered
something, which I consider a bug (it appears in 1.5.1 release and in a
latest version from CVS head).
The problem is when I call method "suspendProcessing()" followed by
"dispose()" on JSVGCanvas object,
the JSVGCanvas won't be collected by garbage collector. This is somehow
connected with UpdateManager ant its RunnableQueue, I think.
This is tied to the following bug:
http://issues.apache.org/bugzilla/show_bug.cgi?id=30189

I think I have a solution to this bug. This makes it clear
why suspend/dispose isn't shutting down the UpdateManager Thread
properly, but I am unsure why this would prevent GC (this is
also troubling). My fix for the suspend/resume race condition
does allow the Canvas to go to GC.

One question on this, what events should be sent? when
suspend and resume are called in close proximity (i.e. before
the runnable thread actually suspends). Right now no thread
events get sent (just like if you call resume when the thread
is already running).
Post by Lukasz Matuszczak
You may ask why i call dispose() after suspendProcessing() (why not only
dispose()). The answer is that I have my own queue of events in my
application, and sometimes I get a request to deactivate the SVG screen
(suspendProcessing), and soon after that to close it (close). I handle
these events one after another (of course AWT in event dispatch thread).
Sure, this is an expected usecase.

Thanks for the good test case.
Lukasz Matuszczak
2005-02-21 07:11:21 UTC
Permalink
----- Original Message -----
From: "Thomas DeWeese" <***@Kodak.com>
To: "Batik Users" <batik-***@xml.apache.org>
Sent: Friday, February 18, 2005 1:01 PM
Subject: Re: suspendProcessing() and dispose() memory leak


Hi Thomas,
Post by Thomas DeWeese
Hi Lukasz,
Post by Lukasz Matuszczak
I am still fighting with "dispose()" problems, and I discovered something,
which I consider a bug (it appears in 1.5.1 release and in a latest version
from CVS head).
The problem is when I call method "suspendProcessing()" followed by
"dispose()" on JSVGCanvas object,
the JSVGCanvas won't be collected by garbage collector. This is somehow
connected with UpdateManager ant its RunnableQueue, I think.
http://issues.apache.org/bugzilla/show_bug.cgi?id=30189
I think I have a solution to this bug. This makes it clear
why suspend/dispose isn't shutting down the UpdateManager Thread
properly, but I am unsure why this would prevent GC (this is
also troubling). My fix for the suspend/resume race condition
does allow the Canvas to go to GC.
Thank you very much, Thomas, your solution seems to work.
Post by Thomas DeWeese
One question on this, what events should be sent? when
suspend and resume are called in close proximity (i.e. before
the runnable thread actually suspends). Right now no thread
events get sent (just like if you call resume when the thread
is already running).
I think when we call suspend or resume we should always get managerSuspended and
managerResumed events respectively, even if they are called in close proximity
(unless the manager fails). Does it work this way in a current solution?

Best regards,
Lukasz
Thomas DeWeese
2005-02-21 11:15:22 UTC
Permalink
Post by Lukasz Matuszczak
Post by Thomas DeWeese
I think I have a solution to this bug.
Hi Lukasz,
Post by Lukasz Matuszczak
Thank you very much, Thomas, your solution seems to work.
Good Glad to hear it.
Post by Lukasz Matuszczak
Post by Thomas DeWeese
One question on this, what events should be sent? when
suspend and resume are called in close proximity (i.e. before
the runnable thread actually suspends). Right now no thread
events get sent (just like if you call resume when the thread
is already running).
I think when we call suspend or resume we should always get
managerSuspended and managerResumed events respectively, even if they
are called in close proximity
I think it's a little misleading to send a "managerSuspended"
event when the manager is still running don't you? Even in the
past if you called 'resume' when it was already running no event
would be fired. So the answer isn't quite as obvious as one might
think.
Post by Lukasz Matuszczak
(unless the manager fails). Does it work this way in a current solution?
No, currently if the runnable queue doesn't actually stop before you
call resume it will not send either of the events. What I will probably
try and do is have it send a resumed event (which could be considered
a failed event for the suspend request). This is a little tricky to
do right.
Archie Cobbs
2005-02-21 15:29:32 UTC
Permalink
Post by Thomas DeWeese
Post by Lukasz Matuszczak
I think when we call suspend or resume we should always get
managerSuspended and managerResumed events respectively, even if they
are called in close proximity
I think it's a little misleading to send a "managerSuspended"
event when the manager is still running don't you? Even in the
past if you called 'resume' when it was already running no event
would be fired. So the answer isn't quite as obvious as one might
think.
Either way, proper documentation should save the day. E.g., with the
current behavior the API for suspend() and resume() should say that
they are "requests" to suspend/resume the update manager, and that
multiple requests can get consolidated, and that the listener events
are reports of what actually happened, etc.

-Archie

__________________________________________________________________________
Archie Cobbs * CTO, Awarix * http://www.awarix.com
Loading...