This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 125149 - RaveText/RaveElement memory leaks in RaveRenderedDocument.userData
Summary: RaveText/RaveElement memory leaks in RaveRenderedDocument.userData
Status: RESOLVED FIXED
Alias: None
Product: obsolete
Classification: Unclassified
Component: visualweb (show other bugs)
Version: 6.x
Hardware: All All
: P2 blocker (vote)
Assignee: Peter Zavadsky
URL:
Keywords: PERFORMANCE
Depends on:
Blocks: 123530
  Show dependency tree
 
Reported: 2008-01-12 00:35 UTC by Quy Nguyen
Modified: 2008-01-23 22:45 UTC (History)
0 users

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Quy Nguyen 2008-01-12 00:35:25 UTC
Path from RaveRenderedDocument -> RaveText:

RaveRenderedDocument.userData -> 
Hashtable.Entry.key ->
org.apache.xerces.dom.AttributeMap.nodes ->
AbstractRaveDocument.Fix105085AttrImpl.value ->
RaveText

Path from RaveRenderedDocument -> RaveElement

RaveRenderedDocument.userData -> 
Hashtable.Entry.key ->
RaveElement


To reproduce:
1) Create a vw project
2) Add a large number of components to the page
3) Switch to JSP view
4) Make a modification
5) Switch to Design view
6) Delete all components
7) Switch to JSP view
8) Make a modification
9) Switch to Design view

Steps 3-5 and 7-9 may need to be executed multiple times before the memory leak occurs.
Comment 1 Peter Zavadsky 2008-01-14 19:20:57 UTC
Yes, there needs to be replaced usages of userData, they leak in the apache Document implementation.
Comment 2 Peter Zavadsky 2008-01-14 23:52:24 UTC
First part of the fix, removed usage of userData in WebForm.

Checking in visualweb/designer/src/org/netbeans/modules/visualweb/designer/WebForm.java;
/cvs/visualweb/designer/src/org/netbeans/modules/visualweb/designer/WebForm.java,v  <--  WebForm.java
new revision: 1.185; previous revision: 1.184
done
Comment 3 Peter Zavadsky 2008-01-15 00:16:49 UTC
Another part:

Checking in visualweb/designer/markup/src/org/netbeans/modules/visualweb/designer/markup/MarkupServiceImpl.java;
/cvs/visualweb/designer/markup/src/org/netbeans/modules/visualweb/designer/markup/MarkupServiceImpl.java,v  <-- 
MarkupServiceImpl.java
new revision: 1.17; previous revision: 1.16
done
Comment 4 Peter Zavadsky 2008-01-15 00:19:17 UTC
Checking in visualweb/designer/markup/src/org/netbeans/modules/visualweb/designer/markup/MarkupServiceImpl.java;
/cvs/visualweb/designer/markup/src/org/netbeans/modules/visualweb/designer/markup/MarkupServiceImpl.java,v  <-- 
MarkupServiceImpl.java
new revision: 1.18; previous revision: 1.17
done

Comment 5 Peter Zavadsky 2008-01-15 00:24:55 UTC
Checking in visualweb/designer/markup/src/org/netbeans/modules/visualweb/designer/markup/MarkupServiceImpl.java;
/cvs/visualweb/designer/markup/src/org/netbeans/modules/visualweb/designer/markup/MarkupServiceImpl.java,v  <-- 
MarkupServiceImpl.java
new revision: 1.19; previous revision: 1.18
done
Comment 6 Peter Zavadsky 2008-01-15 19:00:40 UTC
Fixing in insync module.

Checking in visualweb/insync/src/org/netbeans/modules/visualweb/insync/faces/FacesPageUnit.java;
/cvs/visualweb/insync/src/org/netbeans/modules/visualweb/insync/faces/FacesPageUnit.java,v  <--  FacesPageUnit.java
new revision: 1.18; previous revision: 1.17
done

Checking in visualweb/insync/src/org/netbeans/modules/visualweb/insync/markup/MarkupUnit.java;
/cvs/visualweb/insync/src/org/netbeans/modules/visualweb/insync/markup/MarkupUnit.java,v  <--  MarkupUnit.java
new revision: 1.11; previous revision: 1.10
done
Comment 7 Peter Zavadsky 2008-01-15 19:34:06 UTC
Last case found, fixed.
Checking in visualweb/designer/cssengine/src/org/netbeans/modules/visualweb/designer/cssengine/CssEngineServiceImpl.java;
/cvs/visualweb/designer/cssengine/src/org/netbeans/modules/visualweb/designer/cssengine/CssEngineServiceImpl.java,v  <--
 CssEngineServiceImpl.java
new revision: 1.19; previous revision: 1.18
done

Now, there shouldn't be userData used anymore (from our modules). Please, check whether those weak hash maps are enough
now, or whether there is needed to change to mapping to some weak refs (within those maps).
Comment 8 Quy Nguyen 2008-01-15 23:35:53 UTC
The WeakHashMap entries do not appear to get cleared when running the test case from the issue description.  There are
probably strong references from the WeakHashMap values to the keys (which are not visible in a heap dump).
Comment 9 Peter Zavadsky 2008-01-15 23:38:54 UTC
Please, provide, which WeakHashMap instances you refer to.
Comment 10 Quy Nguyen 2008-01-16 00:08:00 UTC
Some of the maps that I have seen leaking are:

- CssEngineServiceImpl.doc2engine (which leaks RaveSourceDocument objects)
- MarkupServiceImpl.element2cssStylableElement

Comment 11 Peter Zavadsky 2008-01-16 01:39:34 UTC
First fixed the MarkupServiceImpl.element2cssStylableElement weak hash map.
The other one CssEngineServiceImpl.doc2engine is a bit harder, because the map also serves as a holder of the engine. I
need to change the references to document from the engine impl (Batik code).
Comment 12 Peter Zavadsky 2008-01-16 18:43:16 UTC
Fixed. Now also the mapping document to css engine should work.

Checking in visualweb/designer/cssengine/src/org/netbeans/modules/visualweb/designer/cssengine/CssEngineServiceImpl.java;
/cvs/visualweb/designer/cssengine/src/org/netbeans/modules/visualweb/designer/cssengine/CssEngineServiceImpl.java,v  <--
 CssEngineServiceImpl.java
new revision: 1.20; previous revision: 1.19
done
Comment 13 Quy Nguyen 2008-01-16 22:52:56 UTC
One of the other WeakHashMap instances is causing a memory leak as well:

[#document: null]:
private static final java.util.Map org.netbeans.modules.visualweb.designer.cssengine.CssEngineServiceImpl.engineKey2engine->
java.util.WeakHashMap@62b644-table->
[Ljava.util.WeakHashMap$Entry;@41502d-[8]->
java.util.WeakHashMap$Entry@1069d5-value->
org.netbeans.modules.visualweb.designer.cssengine.XhtmlCssEngine@81d3f4-document->
org.netbeans.modules.visualweb.designer.markup.RaveSourceDocument@ff1f43
Comment 14 Peter Zavadsky 2008-01-17 20:33:52 UTC
That shoud not be correct. The engine  doesn't contain ref to engineKey (so the weak hash map can hold it if the engine
key is free). The engineKey is held by the document (via weak hash map again). I.e. somebody else need to hold on the
document.

Please, provide steps to reproduce, those are missing here.
Comment 15 Quy Nguyen 2008-01-17 20:57:39 UTC
The steps to reproduce are the same as in the original issue description on the top.  From the profiling tools
(heapwalker, etc.), the WeakHashMap entries are not deallocated.
Comment 16 Peter Zavadsky 2008-01-17 21:52:06 UTC
Hm, might be that chaining of those weak hash maps doesn't help. I need to check that. If it is so, than some other kind
of hack will be needed.
Comment 17 Peter Zavadsky 2008-01-17 22:14:30 UTC
OK, I just checked, it seems that the chaining of the weak hash maps don't work. Thinking about how to by pass the issue.
Comment 18 Peter Zavadsky 2008-01-18 00:42:17 UTC
Fixed.
Switched back to userData to keep link between document and css engine.
It seems that the userData work OK when used on the document instance (in contrast to the elements).

Checking in visualweb/designer/cssengine/src/org/netbeans/modules/visualweb/designer/cssengine/CssEngineServiceImpl.java;
/cvs/visualweb/designer/cssengine/src/org/netbeans/modules/visualweb/designer/cssengine/CssEngineServiceImpl.java,v  <--
 CssEngineServiceImpl.java
new revision: 1.21; previous revision: 1.20
done
Comment 19 Quy Nguyen 2008-01-18 03:27:42 UTC
The current fixes appear to work correctly.  However, I see another leak (with the same steps to reproduce) from the
RaveRenderedDocument.eventListeners field.  The reference graph is approximately:

RaveRenderedDocument.eventListeners ->
Hashtable.entry.value ->
Vector.elementData ->
org.apache.xerces.dom.DocumentImpl$LEntry.listener->
org.apache.batik.css.engine.CSSEngine$DOMAttrModifiedListener.this$0 ->
XhtmlCssEngine.styleSheetNodes ->
RenderedStylesheetLinkElement.ownerNode ->
RaveElement.nextSibling ->
RaveText
Comment 20 Peter Zavadsky 2008-01-18 19:25:06 UTC
It means those event listeners are the Batik ones (in CSSEngine class). Will look at, what is possible to be done with them.
Comment 21 Peter Zavadsky 2008-01-18 20:00:19 UTC
Ideally there would be a weak listener needed. But there are issues (Batik not dependent on NB, also Dom listener
doesn't follow the rules, it doesn't extend java.util.EventListener).
Comment 22 Peter Zavadsky 2008-01-18 23:34:25 UTC
Quy, I was thinking about this, and there shouldn't be a leak caused by this listener, if nobody holds the document,
this listener won't cause it to be kept, because the css engine should also get garbaged (it should be held only by the
document itself).

Also your example points to the RaveRenderedDocument (not source), which there ought to be 1 instance of it, and that is
correct, right?

Or did you try to point to something else?
Comment 23 Quy Nguyen 2008-01-19 00:19:40 UTC
There is only one RaveRenderedDocument object, but that is correct if the page is still open.  The issue here is that
there is a leak of RaveText objects when repeatedly making modifications to a single page by adding components and then
deleting them.  After a series of modifications, the page returns to its original state (empty), but there are more
RaveText objects in memory.

This should be easily testable since probes for RaveText are already in place, so you can see the number of live
instances in the runtime watches window.
Comment 24 Peter Zavadsky 2008-01-19 01:24:34 UTC
OK, then it needs to be freed the RaveText, but that is not caused by the listener, as long as there is still the same
css engine instance.

So the steps are to add the text nodes, and remove them? Right?
Comment 25 Quy Nguyen 2008-01-19 01:38:16 UTC
I do the following:

1) Add a button component
2) Switch to JSP
3) Make a modification
4) Switch back to designer
5) Delete the button

Doing these steps repeatedly causes a memory leak that can be seen in the runtime watches window (under 'Important
Instances').  The 'find refs' action on any of the probed visual web objects should show you gc roots.
Comment 26 Peter Zavadsky 2008-01-19 02:38:52 UTC
OK, I had a look at it, the path is as you say, but the problem is not in the listener, that one should still be
present. But it seems to me in that CssEngine.styleSheetNodes field, it keeps nodes, which are also supposed to be
deleted. Looking into it.
Comment 27 Peter Zavadsky 2008-01-23 22:45:43 UTC
Fixed the leak in CSSEngine.styleSheetNodes. Now there wasn't left the button element (for that case).

Checking in visualweb/ravelibs/batik/library/nbproject/build-impl.xml;
/cvs/visualweb/ravelibs/batik/library/nbproject/build-impl.xml,v  <--  build-impl.xml
new revision: 1.5; previous revision: 1.4
done
Checking in visualweb/ravelibs/batik/library/nbproject/genfiles.properties;
/cvs/visualweb/ravelibs/batik/library/nbproject/genfiles.properties,v  <--  genfiles.properties
new revision: 1.5; previous revision: 1.4
done
Checking in visualweb/ravelibs/batik/library/nbproject/project.properties;
/cvs/visualweb/ravelibs/batik/library/nbproject/project.properties,v  <--  project.properties
new revision: 1.4; previous revision: 1.3
done
Checking in visualweb/ravelibs/batik/library/src/org/apache/batik/css/engine/CSSEngine.java;
/cvs/visualweb/ravelibs/batik/library/src/org/apache/batik/css/engine/CSSEngine.java,v  <--  CSSEngine.java
new revision: 1.2; previous revision: 1.1
done
Checking in visualweb/ravelibs/batik/nbproject/project.properties;
/cvs/visualweb/ravelibs/batik/nbproject/project.properties,v  <--  project.properties
new revision: 1.4; previous revision: 1.3
done