Bug 132557 - SVG animation stops running.
Summary: SVG animation stops running.
Status: RESOLVED DUPLICATE of bug 191237
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL: http://jsfiddle.net/anvaka/CGcZ6/2/show/
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-05-04 21:45 PDT by Andrei
Modified: 2019-05-06 14:57 PDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrei 2014-05-04 21:45:16 PDT
I'm animating an SVG circle using circle.cx.baseVal property. It works fine when you try to directly access it:

circle.cx.baseVal.value = x;

But it stops working if you do something like this:

var update = (function (circle) {
  var pos = circle.cx.baseVal;
  return function (x) { pos.value = x; }
})(circleElement);

Pattern is similar in all cases. Dot moves for couple frames, then just stops. As soon as I replace pos.value with circle.cx.baseVal.value - all works fine. WebKit nightly requires more time to get it reproduced (usually happens within first 60 seconds). Release version of Safari reproduces it within first couple seconds.

Judging from this pattern, It seems it's GC related

Steps to Reproduce:
Navigate to http://jsfiddle.net/anvaka/CGcZ6/2/show/ - observe dot stops moving.
Source code: http://jsfiddle.net/anvaka/CGcZ6/2/

Expected Results:
Chrome and Firefox keep moving point on the screen indefinitely long.

Actual Results:
Point stops moving

Version:
Safari: Version 6.1.2 (7537.74.9), Version 7.0.2 (9537.74.9), Version 7.0.2 (9537.74.9, 538+)
OSX: 10.7.5, 10.9.2, iOS 7.0.6
Comment 1 Benjamin Poulain 2014-05-10 16:27:12 PDT
Interesting case.

I can reproduce on the released version of WebKit.
On nightly, it is a lot harder to reproduce, it can take up to several minutes on my machine.

It looks like the wrapper for SVGLength does not hold onto its DOM counterpart.
Comment 2 Benjamin Poulain 2014-05-11 13:28:21 PDT
<rdar://problem/16397744>
Comment 3 Benjamin Poulain 2014-05-11 14:41:41 PDT
What happens here is the SVGAnimatedLength is garbage collected, which in turn cause the SVGLength to be detached from the SVGElement:

  * frame #0: 0x0000000109ed92e9 WebCore`WebCore::SVGAnimatedPropertyTearOff<WebCore::SVGLength>::~SVGAnimatedPropertyTearOff() [inlined] WebCore::SVGAnimatedPropertyTearOff<WebCore::SVGLength>::~SVGAnimatedPropertyTearOff(this=0x000000010dda80c0) at SVGAnimatedPropertyTearOff.h:35
    frame #1: 0x0000000109ed92e9 WebCore`WebCore::SVGAnimatedPropertyTearOff<WebCore::SVGLength>::~SVGAnimatedPropertyTearOff() [inlined] WebCore::SVGAnimatedPropertyTearOff<WebCore::SVGLength>::~SVGAnimatedPropertyTearOff(this=0x000000010dda80c0, p=<unavailable>) at SVGAnimatedPropertyTearOff.h:35
    frame #2: 0x0000000109ed92e9 WebCore`WebCore::SVGAnimatedPropertyTearOff<WebCore::SVGLength>::~SVGAnimatedPropertyTearOff(this=0x000000010dda80c0) + 9 at SVGAnimatedPropertyTearOff.h:35
    frame #3: 0x0000000109ecb9f5 WebCore`WebCore::JSSVGAnimatedLengthOwner::finalize(JSC::Handle<JSC::Unknown>, void*) [inlined] WTF::RefCounted<WebCore::SVGAnimatedProperty>::deref(this=<unavailable>) + 213 at RefCounted.h:146
    frame #4: 0x0000000109ecb9e1 WebCore`WebCore::JSSVGAnimatedLengthOwner::finalize(JSC::Handle<JSC::Unknown>, void*) [inlined] WebCore::JSSVGAnimatedLength::releaseImpl() + 11 at JSSVGAnimatedLength.h:56
    frame #5: 0x0000000109ecb9d6 WebCore`WebCore::JSSVGAnimatedLengthOwner::finalize(this=<unavailable>, context=<unavailable>, handle=<unavailable>) + 182 at JSSVGAnimatedLength.cpp:185
    frame #6: 0x000000010bf98408 JavaScriptCore`JSC::WeakBlock::sweep() [inlined] JSC::WeakBlock::finalize(JSC::WeakImpl*) + 104 at WeakSetInlines.h:52
    frame #7: 0x000000010bf983e2 JavaScriptCore`JSC::WeakBlock::sweep(this=0x000000010cdd5000) + 66 at WeakBlock.cpp:76
    frame #8: 0x000000010bf9a088 JavaScriptCore`JSC::WeakSet::sweep(this=0x000000010ca60448) + 40 at WeakSet.cpp:48
    frame #9: 0x000000010bea0de9 JavaScriptCore`JSC::MarkedBlock::sweep(this=0x000000010ca60000, sweepMode=SweepOnly) + 25 at MarkedBlock.cpp:109
    frame #10: 0x000000010bd57769 JavaScriptCore`JSC::IncrementalSweeper::doSweep(double) [inlined] JSC::IncrementalSweeper::sweepNextBlock(this=0x00007fe42060f2d0) + 52 at IncrementalSweeper.cpp:100
    frame #11: 0x000000010bd57735 JavaScriptCore`JSC::IncrementalSweeper::doSweep(this=0x00007fe42060f2d0, sweepBeginTime=4826.326385675) + 101 at IncrementalSweeper.cpp:78
    frame #12: 0x000000010bd56b6a JavaScriptCore`JSC::HeapTimer::timerDidFire(timer=<unavailable>, context=0x00000001077e6bd0) + 170 at HeapTimer.cpp:99

It is not a bug in the Garbage Collector but in the relation between SVGAnimatedProperty and SVGLength. SVGAnimatedProperty needs to be kept alive as long as there is a live wrapper for SVGLength.

Dirk, any opinion? Do you want to take the bug?
Comment 4 Dirk Schulze 2014-05-12 10:40:38 PDT
(In reply to comment #3)
> What happens here is the SVGAnimatedLength is garbage collected, which in turn cause the SVGLength to be detached from the SVGElement:
> 
>   * frame #0: 0x0000000109ed92e9 WebCore`WebCore::SVGAnimatedPropertyTearOff<WebCore::SVGLength>::~SVGAnimatedPropertyTearOff() [inlined] WebCore::SVGAnimatedPropertyTearOff<WebCore::SVGLength>::~SVGAnimatedPropertyTearOff(this=0x000000010dda80c0) at SVGAnimatedPropertyTearOff.h:35
>     frame #1: 0x0000000109ed92e9 WebCore`WebCore::SVGAnimatedPropertyTearOff<WebCore::SVGLength>::~SVGAnimatedPropertyTearOff() [inlined] WebCore::SVGAnimatedPropertyTearOff<WebCore::SVGLength>::~SVGAnimatedPropertyTearOff(this=0x000000010dda80c0, p=<unavailable>) at SVGAnimatedPropertyTearOff.h:35
>     frame #2: 0x0000000109ed92e9 WebCore`WebCore::SVGAnimatedPropertyTearOff<WebCore::SVGLength>::~SVGAnimatedPropertyTearOff(this=0x000000010dda80c0) + 9 at SVGAnimatedPropertyTearOff.h:35
>     frame #3: 0x0000000109ecb9f5 WebCore`WebCore::JSSVGAnimatedLengthOwner::finalize(JSC::Handle<JSC::Unknown>, void*) [inlined] WTF::RefCounted<WebCore::SVGAnimatedProperty>::deref(this=<unavailable>) + 213 at RefCounted.h:146
>     frame #4: 0x0000000109ecb9e1 WebCore`WebCore::JSSVGAnimatedLengthOwner::finalize(JSC::Handle<JSC::Unknown>, void*) [inlined] WebCore::JSSVGAnimatedLength::releaseImpl() + 11 at JSSVGAnimatedLength.h:56
>     frame #5: 0x0000000109ecb9d6 WebCore`WebCore::JSSVGAnimatedLengthOwner::finalize(this=<unavailable>, context=<unavailable>, handle=<unavailable>) + 182 at JSSVGAnimatedLength.cpp:185
>     frame #6: 0x000000010bf98408 JavaScriptCore`JSC::WeakBlock::sweep() [inlined] JSC::WeakBlock::finalize(JSC::WeakImpl*) + 104 at WeakSetInlines.h:52
>     frame #7: 0x000000010bf983e2 JavaScriptCore`JSC::WeakBlock::sweep(this=0x000000010cdd5000) + 66 at WeakBlock.cpp:76
>     frame #8: 0x000000010bf9a088 JavaScriptCore`JSC::WeakSet::sweep(this=0x000000010ca60448) + 40 at WeakSet.cpp:48
>     frame #9: 0x000000010bea0de9 JavaScriptCore`JSC::MarkedBlock::sweep(this=0x000000010ca60000, sweepMode=SweepOnly) + 25 at MarkedBlock.cpp:109
>     frame #10: 0x000000010bd57769 JavaScriptCore`JSC::IncrementalSweeper::doSweep(double) [inlined] JSC::IncrementalSweeper::sweepNextBlock(this=0x00007fe42060f2d0) + 52 at IncrementalSweeper.cpp:100
>     frame #11: 0x000000010bd57735 JavaScriptCore`JSC::IncrementalSweeper::doSweep(this=0x00007fe42060f2d0, sweepBeginTime=4826.326385675) + 101 at IncrementalSweeper.cpp:78
>     frame #12: 0x000000010bd56b6a JavaScriptCore`JSC::HeapTimer::timerDidFire(timer=<unavailable>, context=0x00000001077e6bd0) + 170 at HeapTimer.cpp:99
> 
> It is not a bug in the Garbage Collector but in the relation between SVGAnimatedProperty and SVGLength. SVGAnimatedProperty needs to be kept alive as long as there is a live wrapper for SVGLength.
> 
> Dirk, any opinion? Do you want to take the bug?

I am absolutely not familiar with the code (partly: anymore). I personally would rather not like to hack in this code base. Instead I would like to disable the animation syntonization code. But not sure if that is still possible or if people rely on it. I certainly hope no one uses animVal and we can remove SVGAnimatedPropertyTearOff
Comment 5 Said Abou-Hallawa 2019-05-06 14:57:33 PDT
I believe this bug is fixed after https://bugs.webkit.org/show_bug.cgi?id=191237 is fixed. The tear-off objects were removed. SVGLength does not depend on the existence of SVGAnimatedLength anymore.

*** This bug has been marked as a duplicate of bug 191237 ***