You are viewing a plain text version of this content. The canonical link for it is here.
Posted to batik-dev@xmlgraphics.apache.org by th...@kodak.com on 2006/02/17 12:34:36 UTC

Re: svn commit: r377607 - in /xmlgraphics/batik/trunk/sources/org/apache/batik: bridge/UpdateManager.java script/rhino/RhinoClassLoader.java

Hi Cameron,

   I'm worried that these represent a very significant
Memory leak.  How are these managed as documents go
into/out of scope? (think someone twiddling the
xlink:href on an image/use element).

cam@apache.org wrote on 02/13/2006 11:09:59 PM:

>      /**
> +     * Array of resource documents' BridgeContexts.
> +     */
> +    protected BridgeContext[] secondaryBridgeContexts;
> +
> +    /**
> +     * Array of resource documents' ScriptingEnvironments that should
> +     * have their SVGLoad event dispatched.
> +     */
> +    protected ScriptingEnvironment[] secondaryScriptingEnvironments;

> +        secondaryBridgeContexts =
> +            (BridgeContext[]) ctx.getChildContexts().clone();
> +        secondaryScriptingEnvironments =
> +            new ScriptingEnvironment[secondaryBridgeContexts.length];
> +        for (int i = 0; i < secondaryBridgeContexts.length; i++) {
> +            BridgeContext resCtx = secondaryBridgeContexts[i];
> +            if (!((SVGOMDocument) resCtx.getDocument()).isSVG12()) {
> +                continue;
> +            }
>              resCtx.setUpdateManager(this);
> -            initializeScriptingEnvironment(resCtx);
> +            ScriptingEnvironment se = 
initializeScriptingEnvironment(resCtx);
> +            secondaryScriptingEnvironments[i] = se;


---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org


Re: svn commit: r377607 - in /xmlgraphics/batik/trunk/sources/org/apache/batik: bridge/UpdateManager.java script/rhino/RhinoClassLoader.java

Posted by Cameron McCormack <ca...@mcc.id.au>.
Hi Thomas.

thomas.deweese@kodak.com:
>    I'm worried that these represent a very significant
> Memory leak.  How are these managed as documents go
> into/out of scope? (think someone twiddling the
> xlink:href on an image/use element).

You may be right, since I don't reference count the documents.  The
array is only cleared when the primary BridgeContext is disposed.  In
SVG 1.2 documents perhaps they should be kept around though, since they
have their own script engines still running that may do something.
Would soft references here be sufficient to allow a resource document
to be freed if it is unreferenced?

-- 
 Cameron McCormack			ICQ: 26955922
 cam (at) mcc.id.au			MSN: cam (at) mcc.id.au
 http://mcc.id.au/			JBR: heycam (at) jabber.org

---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org


Re: Updating rhino (was: svn commit: ...)

Posted by Cameron McCormack <ca...@mcc.id.au>.
thomas.deweese@kodak.com:
>    This is tricky since the NPL/MPL has some kind of 'recontribution'
> requirement... 
> 
>    What does the patch look like?  Is it significant? or twiddling 
> public/private adding accessor type stuff?  Can you put up either on 
> the Wiki or Bugzilla (you could use theirs I suppose)?

The patch I submitted to them involved some accessor twiddling and
accessor method additions.  Also a couple of methods to allow the
debugger to detach from a script.  The patch itself is quite huge,
though, since I also cleaned up and reordered the code a bit and added
some javadoc comments.  Unfortunately that kind of obscures the
important changes I made to the code.

The patch is here:

  https://bugzilla.mozilla.org/attachment.cgi?id=214133&action=view

-- 
 Cameron McCormack			ICQ: 26955922
 cam (at) mcc.id.au			MSN: cam (at) mcc.id.au
 http://mcc.id.au/			JBR: heycam (at) jabber.org

---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org


Re: Updating rhino (was: svn commit: ...)

Posted by th...@kodak.com.
Hi Cameron,

Cameron McCormack <ca...@mcc.id.au> wrote on 03/05/2006 07:55:33 PM:

> Ok, I have made changes that lets Rhino 1.6R2 work well with Batik.
> However, there were some changes to the debugger which made it
> impossible to use without modification.  I submitted a patch to the
> Rhino people, but I don't know if/when it will be accepted.  Is it OK to
> include my patched CVS snapshot of js.jar in the repository?  Should I
> somehow keep the modifications in there, too?

   This is tricky since the NPL/MPL has some kind of 'recontribution'
requirement... 

   What does the patch look like?  Is it significant? or twiddling 
public/private adding accessor type stuff?  Can you put up either on 
the Wiki or Bugzilla (you could use theirs I suppose)?


---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org


Updating rhino (was: svn commit: ...)

Posted by Cameron McCormack <ca...@mcc.id.au>.
Cameron McCormack:
[... about updating rhino ...]
> > If not I'll do some testing and if all is well, update the jar 
> > in the respository.

Thomas Deweese:
>     Sounds good to me.

Ok, I have made changes that lets Rhino 1.6R2 work well with Batik.
However, there were some changes to the debugger which made it
impossible to use without modification.  I submitted a patch to the
Rhino people, but I don't know if/when it will be accepted.  Is it OK to
include my patched CVS snapshot of js.jar in the repository?  Should I
somehow keep the modifications in there, too?

Thanks,

Cameron

-- 
 Cameron McCormack			ICQ: 26955922
 cam (at) mcc.id.au			MSN: cam (at) mcc.id.au
 http://mcc.id.au/			JBR: heycam (at) jabber.org

---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org


Re: svn commit: r377607 - in /xmlgraphics/batik/trunk/sources/org/apache/batik: bridge/UpdateManager.java script/rhino/RhinoClassLoader.java

Posted by th...@kodak.com.
Hi Cameron,

Cameron McCormack <ca...@mcc.id.au> wrote on 02/20/2006 07:44:03 PM:

> thomas.deweese@kodak.com:
> >    We might also check with the Rhino folks to see if they have
> > any suggestions on how to avoid this problem.
> 
> It looks like this was fixed in 1.5R5:
> 
>   https://bugzilla.mozilla.org/show_bug.cgi?id=213279

   Cool!

> Do you know of any issues preventing an upgrade of Rhino? 

   I don't know of anything concrete.  However we do get into
it pretty deep in places.

> If not I'll do some testing and if all is well, update the jar 
> in the respository.

    Sounds good to me.


---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org


Re: svn commit: r377607 - in /xmlgraphics/batik/trunk/sources/org/apache/batik: bridge/UpdateManager.java script/rhino/RhinoClassLoader.java

Posted by Cameron McCormack <ca...@mcc.id.au>.
Hi Thomas.

thomas.deweese@kodak.com:
>    We might also check with the Rhino folks to see if they have
> any suggestions on how to avoid this problem.

It looks like this was fixed in 1.5R5:

  https://bugzilla.mozilla.org/show_bug.cgi?id=213279

Do you know of any issues preventing an upgrade of Rhino?  If not I'll
do some testing and if all is well, update the jar in the respository.

Thanks,

Cameron

-- 
 Cameron McCormack			ICQ: 26955922
 cam (at) mcc.id.au			MSN: cam (at) mcc.id.au
 http://mcc.id.au/			JBR: heycam (at) jabber.org

---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org


Re: svn commit: r377607 - in /xmlgraphics/batik/trunk/sources/org/apache/batik: bridge/UpdateManager.java script/rhino/RhinoClassLoader.java

Posted by th...@kodak.com.
Hi Cameron,

Cameron McCormack <ca...@mcc.id.au> wrote on 02/20/2006 02:51:05 AM:

> thomas.deweese@kodak.com:
> >    I'm worried that these represent a very significant
> > Memory leak.  How are these managed as documents go
> > into/out of scope? (think someone twiddling the
> > xlink:href on an image/use element).
> 
> Ok I have investigated this a bit more.  Using WeakReferences in the
> childContexts set does avoid the problem of the resource documents not
> being GCed if they are subsequently unreferenced before the document
> ends.

    Good.

> However, there is another problem that I have came across while poring
> over the output from 'hat'.  For the case where a dynamic SVG 1.2
> document is used as a resource document, it won't be GCed because of
> some caching going on by Rhino.[...]
> The "reference chain from rootset excluding weak references" shows that
> the SVG12BridgeContext (and its document) are referenced from a static
> member of org.mozilla.javascript.JavaMembers, which seems to be
> caching various reflections of Java object methods (such as those in
> the document).

   Hmm, this is a problem.  We used to have this problem for all
Documents the solution was to call: 
        Context.setCachingEnabled(false);   // clears the static cache
        Context.setCachingEnabled(true);

   When the 'root' document went out of scope.

   It may be that we need to always run with Caching disabled.
Or else we will need to find a way to be 'notified' when the
image element reference is cleared (perhaps using the 
batik.util.CleanerThread.WeakReferenceCleared class? Although
it's not entirely clear what we would WeakReference as it must
be something that _will_ go to GC when the reference changes 
even if the rest doesn't - we could potentially create such
an item).

   We might also check with the Rhino folks to see if they have
any suggestions on how to avoid this problem.

> The reference comes about because the JS 'parent'
> property of each of these functions is set to the window wrapper
> objects, o.a.b.script.rhino.svg12.GlobalWrapper, and that object has a
> reference to the Window object in the ScriptingEnvironment object.

   Just so you know, the problem is actually wider than this in
general,  IIRC you get references like this in a fairly wide number
of cases where you can't really avoid it (registering event listeners
etc).  So you can't hope to really avoid the problem by making the
'parent' object some sort of weak reference.

> So I'm not sure what to do about this.  Despite what to do for a
> technical solution, I don't know how GC of SVG 1.2 resource documents
> should be handled.  Since they have their own script environments, which
> could be doing any manner of things in the background, and because of
> the text in "Externally referenced documents" in [1], I'm not sure that
> the document should be GCed until the primary document has gone out of
> scope.  Regardless, it would be good to solve the problem with Rhino
> caching.

   Well for my 2cents.  I think any SVG document referenced from the image
element should have it's script run (it is true that the current 1.1 impls
don't do this, but I think this is generally acknowledged as a 
shortfall/bug).
The SVG 1.2 spec makes this clearer. 

   As for when/if they can go to GC the question is how can what they are 
doing in the background be visible to the user?  I personally don't think 
it's worth worrying about a child document 'manipulating' the parent 
document - this is more likely to be an attack than 'correct' behavior. 
If they want 'persistant' script then they should reference the JS from 
the 
parent document.

   I think the child docs should become eligible for GC as soon as the
image reference is changed/removed.


---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org


Re: svn commit: r377607 - in /xmlgraphics/batik/trunk/sources/org/apache/batik: bridge/UpdateManager.java script/rhino/RhinoClassLoader.java

Posted by Cameron McCormack <ca...@mcc.id.au>.
Hi Thomas.

thomas.deweese@kodak.com:
>    I'm worried that these represent a very significant
> Memory leak.  How are these managed as documents go
> into/out of scope? (think someone twiddling the
> xlink:href on an image/use element).

Ok I have investigated this a bit more.  Using WeakReferences in the
childContexts set does avoid the problem of the resource documents not
being GCed if they are subsequently unreferenced before the document
ends.

However, there is another problem that I have came across while poring
over the output from 'hat'.  For the case where a dynamic SVG 1.2
document is used as a resource document, it won't be GCed because of
some caching going on by Rhino.  I've checked in a test that
demonstrates the problem (samples/tests/spec/scripting/primaryDoc.svg,
referenced from test-resources/org/apache/batik/swing/unitTesting.xml).
If you uncomment the script line in primaryDoc.svg you get an
appropriate pause to dump the heap.  Inspecting the heap for instances
of SVG12BridgeContext you'll find the one uncollected one.

The "reference chain from rootset excluding weak references" shows that
the SVG12BridgeContext (and its document) are referenced from a static
member of org.mozilla.javascript.JavaMembers, which seems to be
caching various reflections of Java object methods (such as those in
the document).  The reference comes about because the JS 'parent'
property of each of these functions is set to the window wrapper
objects, o.a.b.script.rhino.svg12.GlobalWrapper, and that object has a
reference to the Window object in the ScriptingEnvironment object.

So I'm not sure what to do about this.  Despite what to do for a
technical solution, I don't know how GC of SVG 1.2 resource documents
should be handled.  Since they have their own script environments, which
could be doing any manner of things in the background, and because of
the text in "Externally referenced documents" in [1], I'm not sure that
the document should be GCed until the primary document has gone out of
scope.  Regardless, it would be good to solve the problem with Rhino
caching.

Thanks,

Cameron

[1] http://www.w3.org/TR/2004/WD-SVG12-20041027/nonvisual.html

-- 
 Cameron McCormack			ICQ: 26955922
 cam (at) mcc.id.au			MSN: cam (at) mcc.id.au
 http://mcc.id.au/			JBR: heycam (at) jabber.org

---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org