Skip to main content

Bug Tracker

Side navigation

#5684 closed enhancement (fixed)

Opened December 19, 2009 08:23PM UTC

Closed September 28, 2011 04:01PM UTC

Last modified March 08, 2012 05:05PM UTC

Effects: exception in animation callback causes endless loop

Reported by: jixxer Owned by: gnarf
Priority: low Milestone: 1.7
Component: effects Version: 1.4a1
Keywords: exception,animation,callback,1.7-discuss Cc:
Blocked by: Blocking:
Description

This bug was (in some sense) already discovered 1 year ago (Ticket #2846), but no action was taken.

If a function, which is passed as callback function to any animation-effect-function (

animate
/
show
/
hide
/...), throws an exception jQuery generates an endless loop (actually it's a function called via a
setInterval
which is never canceled and the
setInterval
keeps running every 13ms(!).

All linenumbers refer to effects.js jQuery 1.4a2

The proposed patch is (XXXX to be replaced by actual ticket number)

401c401
401c401,402
< this.options.complete.call( this.elem );
---
> // ignore exceptions in callback check Ticket #XXXX
> try { this.options.complete.call( this.elem ); } catch () { }

 


More in detail the following things can happen

  • the callback function is called repeatedly forever (unless at a certain time it stops throwing the exception)
  • if multiple elements where being animated the callback function is only called for the first element which finished animating
  • if the reason for the callback function to throw an exception depends on the element which finished animating, the callback is never called for all elements which complete after the first element which caused the exception in the callback
  • all animations started, after such a faulty callback function was triggered, fail to work completely
  • depending on browser + hardware CPU utilization could og to 100% or even whole browser/OS could stall
  • other jQuery code not related to animation continues too function normally

As already pointed out in the other bug ticket the cause for this is that the call to

clearInterval( timerId );
on line 440 in the
jQuery.fx.stop
function is never reached.

It is also clear why this happens if you are familiar with the inner workings of jQuery for animations. The call to

this.options.complete.call( this.elem );
on line 401 in
jQuery.fx.step
throws the exception thus (further up the call stack) the code on the lines 430-435 in
jQuery.fx.tick()
won't ever be reached which means the
timers/jQuery.timers
array is never emptied and
jQuery.fx.stop
never called.

I understand that it is debatable if this fix should even be applied as it masks an exception and the root fault which triggers this bug is caused by the "programmer" and not directly by jQuery.

So if the decision is to not apply this fix both tickets this one and #2846 should be marked as nofix. But we should add a note to the animation documentation that a fault callback function can have this nasty endless "recursion" effect.


Before reporting I tested in Opera 10.10, FF 3.5.6 and IE6.

Opera 10.10 and FF 3.5.6 show the reported behavior.

IE6 seems to degrade in some strange way also fails to run any animations afterwards

Can't test on IE7, IE8, Safari, Chrome, ...

Attachments (1)
  • test_#5684.html (2.1 KB) - added by jixxer December 19, 2009 08:26PM UTC.

    small test case

Change History (25)

Changed October 28, 2010 05:24PM UTC by anonymous comment:1

This bug drove me crazy for weeks. Finally found the problem yesterday and after a while found this ticket. Anyway, here's a simple test case for any skeptics. Just run this in firebug.

$('a:first').hide('blind', function () { console.log('called'); throw new Error('test'); });

Changed October 29, 2010 04:59AM UTC by SlexAxton comment:2

milestone: 1.41.5
priority: minorlow
status: newopen
type: bugenhancement

I created a live test case for this one: http://jsfiddle.net/shAZT/

The problem occurs because there is an interval that is never cleared because you are throwing an error in every iteration of the callback...

While technically, you should catch your errors, it would probably be good for jQuery to be able to handle situations where the programmer fails.

I'll pass this through as an enhancement to 'protect' the user. Exceptions _usually_ mean _your_ code is broken, so I'd suggest wrapping the code that causes this exception in a try catch block. jQuery could do that, but it'd be too expensive to do for all cases, just so the few people who accidentally cause exceptions don't break (more).

We'll look at doing some code restructuring so it doesn't rely on the successful callback of the function.

Thanks for the ticket and test case! Very much appreciated.

Changed November 24, 2010 01:12PM UTC by mail@jasonwoods.me.uk comment:3

Hi all,

I thought maybe one could try returning the callback function from step(). Then if we return true, it means tick along. If we don't, it means end of the animation. We then drop the timer, and see if we returned a callback. If we returned a callback, we call it.

This adds just 2 extra compares - not sure if this is just as expensive as a try catch block but I would assume not.

An additional benefit is the error is still raised for each call of the function - but it doesn't cause a loop and thousands of entries in the error log, so for each instance the function runs and fails, it only fails once and generates a single exception. Only downside to this is the animations may delay slightly because if an exception raises on the first timer, the function fails and other timers won't tick until the next tick. But I consider this perfectly acceptable when an Exception is raised - getting rid of the loop is the most important thing as far as I'm concerned at least!

Here is the modifications I did to the released 1.4.4 file at http://code.jquery.com/jquery-1.4.4.js. If you would like me to apply the changes to a working copy or something let me know :)

(Sorry for dropping the diff directly in the post - thought it was the best place for it as it is only small)

#!diff
6746a

.
6744,6745c

			// #5684 (Driskell) step() returns true for incomplete animation (push to timers)
			// returns false when an animation in a set is completed and it is not the last
			// returns a callback when all animationsin a set are completed
			if ( (complete = timers[i]()) !== true ) {
				timer = timers.splice(i--, 1);
				if (complete !== false) complete.call(timer.elem);
.
6741c
		var timers = jQuery.timers,
			complete, timer;
.
6715,6716c
				// #5684 (Driskell) return the callback function so we can remove the timer before we call it
				// this will ensure any exception returned by the completion function does not cause a loop because the timer was not removed
				return this.options.complete;
.
6645,6647c
		// #5684 (Driskell) step() returns true for incomplete animation (push to timers)
		// returns false when an animation in a set is completed and it is not the last
		// returns a callback when all animationsin a set are completed
		if ( (complete = t()) === true ) {
			if ( jQuery.timers.push(t) && !timerId ) timerId = setInterval(fx.tick, fx.interval);
		} else if (complete !== false) complete.call(this.elem);
.
6630c
			fx = jQuery.fx,
			complete;
.

Let me know your thoughts.

Jason a.k.a. Driskell

Changed November 29, 2010 09:58AM UTC by Jason a.k.a. Driskell <mail@jasonwoods.me.uk> comment:4

Here's new patch file that follows jQuery Core Style Guidelines:

http://www.jasonwoods.me.uk/jquery/jquery-1.4.4.diff

And a prepatched file for those who need:

http://www.jasonwoods.me.uk/jquery/jquery-1.4.4.js

Let me know your thoughts.

Jason a.k.a. Driskell

Changed December 07, 2010 11:31AM UTC by Jason a.k.a. Driskell <mail@jasonwoods.me.uk> comment:5

Hi,

Found an issue with my previous patch on my development server - it didn't work when you called stop() to stop the animations. Also when taking the elem from the splice I missed the subscript so it returned undefined - which made things worse than before :) sorry!

Resolved it all in this new one, which we're now using in production. Let me know if anyone spots any other issues:

Patch for 1.4.4: http://www.jasonwoods.me.uk/jquery/jquery-1.4.4.diff.fixed

Prepatched file: http://www.jasonwoods.me.uk/jquery/jquery-1.4.4.js.fixed

If you need the 1.4.4 I used to create the patch it is here: http://www.jasonwoods.me.uk/jquery/jquery-1.4.4.js.orig

Also if you need to know I ran diff -E to make the patch. If we need different format let me know.

Kind Regards,

Jason a.k.a. Driskell

Changed April 16, 2011 11:45PM UTC by john comment:6

milestone: → 1.next

We should look into this for 1.7.

Changed May 22, 2011 07:27PM UTC by john comment:7

keywords: exception animation callbackexception,animation,callback,1.7-discuss

Nominating ticket for 1.7 discussion.

Changed May 22, 2011 10:25PM UTC by rwaldron comment:8

description: This bug was (in some sense) already discovered 1 year ago (Ticket #2846), but no action was taken. \ \ If a function, which is passed as callback function to any animation-effect-function ({{{animate}}}/{{{show}}}/{{{hide}}}/...), throws an exception jQuery generates an endless loop (actually it's a function called via a {{{setInterval}}} which is never canceled and the {{{setInterval}}} keeps running every 13ms(!). \ \ All linenumbers refer to [http://github.com/jquery/jquery/blob/1.4a2/src/effects.js effects.js jQuery 1.4a2] \ \ The proposed patch is (XXXX to be replaced by actual ticket number) \ \ {{{ \ 401c401 \ 401c401,402 \ < this.options.complete.call( this.elem ); \ --- \ > // ignore exceptions in callback check Ticket #XXXX \ > try { this.options.complete.call( this.elem ); } catch () { } \ \ }}} \ \ ---- \ \ More in detail the following things can happen \ \ - the callback function is called repeatedly forever (unless at a certain time it stops throwing the exception) \ \ - if multiple elements where being animated the callback function is only called for the first element which finished animating \ \ - if the reason for the callback function to throw an exception depends on the element which finished animating, the callback is never called for all elements which complete after the first element which caused the exception in the callback \ \ - all animations started, after such a faulty callback function was triggered, fail to work completely \ \ - depending on browser + hardware CPU utilization could og to 100% or even whole browser/OS could stall \ \ - other jQuery code not related to animation continues too function normally \ \ \ As already pointed out in the other bug ticket the cause for this is that the call to {{{clearInterval( timerId );}}} on line 440 in the {{{jQuery.fx.stop}}} function is never reached. \ \ It is also clear why this happens if you are familiar with the inner workings of jQuery for animations. The call to {{{this.options.complete.call( this.elem );}}} on line 401 in {{{jQuery.fx.step}}} throws the exception thus (further up the call stack) the code on the lines 430-435 in {{{jQuery.fx.tick()}}} won't ever be reached which means the {{{timers/jQuery.timers}}} array is never emptied and {{{jQuery.fx.stop}}} never called. \ \ I understand that it is debatable if this fix should even be applied as it masks an exception and the root fault which triggers this bug is caused by the "programmer" and not directly by jQuery. \ \ So if the decision is to not apply this fix both tickets this one and #2846 should be marked as nofix. But we should add a note to the animation documentation that a fault callback function can have this nasty endless "recursion" effect. \ \ ---- \ \ Before reporting I tested in Opera 10.10, FF 3.5.6 and IE6. \ \ Opera 10.10 and FF 3.5.6 show the reported behavior. \ \ IE6 seems to degrade in some strange way also fails to run any animations afterwards \ \ Can't test on IE7, IE8, Safari, Chrome, ...This bug was (in some sense) already discovered 1 year ago (Ticket #2846), but no action was taken.\ \ If a function, which is passed as callback function to any animation-effect-function ({{{animate}}}/{{{show}}}/{{{hide}}}/...), throws an exception jQuery generates an endless loop (actually it's a function called via a {{{setInterval}}} which is never canceled and the {{{setInterval}}} keeps running every 13ms(!).\ \ All linenumbers refer to [http://github.com/jquery/jquery/blob/1.4a2/src/effects.js effects.js jQuery 1.4a2]\ \ The proposed patch is (XXXX to be replaced by actual ticket number)\ \ {{{\ 401c401\ 401c401,402\ < this.options.complete.call( this.elem );\ ---\ > // ignore exceptions in callback check Ticket #XXXX\ > try { this.options.complete.call( this.elem ); } catch () { }\ \ }}}\ \ ----\ \ More in detail the following things can happen\ \ - the callback function is called repeatedly forever (unless at a certain time it stops throwing the exception)\ \ - if multiple elements where being animated the callback function is only called for the first element which finished animating\ \ - if the reason for the callback function to throw an exception depends on the element which finished animating, the callback is never called for all elements which complete after the first element which caused the exception in the callback\ \ - all animations started, after such a faulty callback function was triggered, fail to work completely\ \ - depending on browser + hardware CPU utilization could og to 100% or even whole browser/OS could stall\ \ - other jQuery code not related to animation continues too function normally\ \ \ As already pointed out in the other bug ticket the cause for this is that the call to {{{clearInterval( timerId );}}} on line 440 in the {{{jQuery.fx.stop}}} function is never reached.\ \ It is also clear why this happens if you are familiar with the inner workings of jQuery for animations. The call to {{{this.options.complete.call( this.elem );}}} on line 401 in {{{jQuery.fx.step}}} throws the exception thus (further up the call stack) the code on the lines 430-435 in {{{jQuery.fx.tick()}}} won't ever be reached which means the {{{timers/jQuery.timers}}} array is never emptied and {{{jQuery.fx.stop}}} never called.\ \ I understand that it is debatable if this fix should even be applied as it masks an exception and the root fault which triggers this bug is caused by the "programmer" and not directly by jQuery.\ \ So if the decision is to not apply this fix both tickets this one and #2846 should be marked as nofix. But we should add a note to the animation documentation that a fault callback function can have this nasty endless "recursion" effect.\ \ ----\ \ Before reporting I tested in Opera 10.10, FF 3.5.6 and IE6.\ \ Opera 10.10 and FF 3.5.6 show the reported behavior.\ \ IE6 seems to degrade in some strange way also fails to run any animations afterwards\ \ Can't test on IE7, IE8, Safari, Chrome, ...

+1, Sounds like a bug, should be fixed

Changed May 22, 2011 11:55PM UTC by jaubourg comment:9

-1, No, no, no, no. Never EVER hide an exception. We could use a try/finally block... but we all know how stupidly unsupported this construct is in old IEs.

Changed May 23, 2011 01:54AM UTC by ajpiano comment:10

description: This bug was (in some sense) already discovered 1 year ago (Ticket #2846), but no action was taken.\ \ If a function, which is passed as callback function to any animation-effect-function ({{{animate}}}/{{{show}}}/{{{hide}}}/...), throws an exception jQuery generates an endless loop (actually it's a function called via a {{{setInterval}}} which is never canceled and the {{{setInterval}}} keeps running every 13ms(!).\ \ All linenumbers refer to [http://github.com/jquery/jquery/blob/1.4a2/src/effects.js effects.js jQuery 1.4a2]\ \ The proposed patch is (XXXX to be replaced by actual ticket number)\ \ {{{\ 401c401\ 401c401,402\ < this.options.complete.call( this.elem );\ ---\ > // ignore exceptions in callback check Ticket #XXXX\ > try { this.options.complete.call( this.elem ); } catch () { }\ \ }}}\ \ ----\ \ More in detail the following things can happen\ \ - the callback function is called repeatedly forever (unless at a certain time it stops throwing the exception)\ \ - if multiple elements where being animated the callback function is only called for the first element which finished animating\ \ - if the reason for the callback function to throw an exception depends on the element which finished animating, the callback is never called for all elements which complete after the first element which caused the exception in the callback\ \ - all animations started, after such a faulty callback function was triggered, fail to work completely\ \ - depending on browser + hardware CPU utilization could og to 100% or even whole browser/OS could stall\ \ - other jQuery code not related to animation continues too function normally\ \ \ As already pointed out in the other bug ticket the cause for this is that the call to {{{clearInterval( timerId );}}} on line 440 in the {{{jQuery.fx.stop}}} function is never reached.\ \ It is also clear why this happens if you are familiar with the inner workings of jQuery for animations. The call to {{{this.options.complete.call( this.elem );}}} on line 401 in {{{jQuery.fx.step}}} throws the exception thus (further up the call stack) the code on the lines 430-435 in {{{jQuery.fx.tick()}}} won't ever be reached which means the {{{timers/jQuery.timers}}} array is never emptied and {{{jQuery.fx.stop}}} never called.\ \ I understand that it is debatable if this fix should even be applied as it masks an exception and the root fault which triggers this bug is caused by the "programmer" and not directly by jQuery.\ \ So if the decision is to not apply this fix both tickets this one and #2846 should be marked as nofix. But we should add a note to the animation documentation that a fault callback function can have this nasty endless "recursion" effect.\ \ ----\ \ Before reporting I tested in Opera 10.10, FF 3.5.6 and IE6.\ \ Opera 10.10 and FF 3.5.6 show the reported behavior.\ \ IE6 seems to degrade in some strange way also fails to run any animations afterwards\ \ Can't test on IE7, IE8, Safari, Chrome, ...This bug was (in some sense) already discovered 1 year ago (Ticket #2846), but no action was taken. \ \ If a function, which is passed as callback function to any animation-effect-function ({{{animate}}}/{{{show}}}/{{{hide}}}/...), throws an exception jQuery generates an endless loop (actually it's a function called via a {{{setInterval}}} which is never canceled and the {{{setInterval}}} keeps running every 13ms(!). \ \ All linenumbers refer to [http://github.com/jquery/jquery/blob/1.4a2/src/effects.js effects.js jQuery 1.4a2] \ \ The proposed patch is (XXXX to be replaced by actual ticket number) \ \ {{{ \ 401c401 \ 401c401,402 \ < this.options.complete.call( this.elem ); \ --- \ > // ignore exceptions in callback check Ticket #XXXX \ > try { this.options.complete.call( this.elem ); } catch () { } \ \ }}} \ \ ---- \ \ More in detail the following things can happen \ \ - the callback function is called repeatedly forever (unless at a certain time it stops throwing the exception) \ \ - if multiple elements where being animated the callback function is only called for the first element which finished animating \ \ - if the reason for the callback function to throw an exception depends on the element which finished animating, the callback is never called for all elements which complete after the first element which caused the exception in the callback \ \ - all animations started, after such a faulty callback function was triggered, fail to work completely \ \ - depending on browser + hardware CPU utilization could og to 100% or even whole browser/OS could stall \ \ - other jQuery code not related to animation continues too function normally \ \ \ As already pointed out in the other bug ticket the cause for this is that the call to {{{clearInterval( timerId );}}} on line 440 in the {{{jQuery.fx.stop}}} function is never reached. \ \ It is also clear why this happens if you are familiar with the inner workings of jQuery for animations. The call to {{{this.options.complete.call( this.elem );}}} on line 401 in {{{jQuery.fx.step}}} throws the exception thus (further up the call stack) the code on the lines 430-435 in {{{jQuery.fx.tick()}}} won't ever be reached which means the {{{timers/jQuery.timers}}} array is never emptied and {{{jQuery.fx.stop}}} never called. \ \ I understand that it is debatable if this fix should even be applied as it masks an exception and the root fault which triggers this bug is caused by the "programmer" and not directly by jQuery. \ \ So if the decision is to not apply this fix both tickets this one and #2846 should be marked as nofix. But we should add a note to the animation documentation that a fault callback function can have this nasty endless "recursion" effect. \ \ ---- \ \ Before reporting I tested in Opera 10.10, FF 3.5.6 and IE6. \ \ Opera 10.10 and FF 3.5.6 show the reported behavior. \ \ IE6 seems to degrade in some strange way also fails to run any animations afterwards \ \ Can't test on IE7, IE8, Safari, Chrome, ...

-1, Agreed with jaubourg, not really into masking the exceptions here.

Changed May 23, 2011 02:57AM UTC by timmywil comment:11

+1, maybe keep the try/catch, throw a custom error, and stop the animation?

Changed May 23, 2011 03:41PM UTC by danheberden comment:12

description: This bug was (in some sense) already discovered 1 year ago (Ticket #2846), but no action was taken. \ \ If a function, which is passed as callback function to any animation-effect-function ({{{animate}}}/{{{show}}}/{{{hide}}}/...), throws an exception jQuery generates an endless loop (actually it's a function called via a {{{setInterval}}} which is never canceled and the {{{setInterval}}} keeps running every 13ms(!). \ \ All linenumbers refer to [http://github.com/jquery/jquery/blob/1.4a2/src/effects.js effects.js jQuery 1.4a2] \ \ The proposed patch is (XXXX to be replaced by actual ticket number) \ \ {{{ \ 401c401 \ 401c401,402 \ < this.options.complete.call( this.elem ); \ --- \ > // ignore exceptions in callback check Ticket #XXXX \ > try { this.options.complete.call( this.elem ); } catch () { } \ \ }}} \ \ ---- \ \ More in detail the following things can happen \ \ - the callback function is called repeatedly forever (unless at a certain time it stops throwing the exception) \ \ - if multiple elements where being animated the callback function is only called for the first element which finished animating \ \ - if the reason for the callback function to throw an exception depends on the element which finished animating, the callback is never called for all elements which complete after the first element which caused the exception in the callback \ \ - all animations started, after such a faulty callback function was triggered, fail to work completely \ \ - depending on browser + hardware CPU utilization could og to 100% or even whole browser/OS could stall \ \ - other jQuery code not related to animation continues too function normally \ \ \ As already pointed out in the other bug ticket the cause for this is that the call to {{{clearInterval( timerId );}}} on line 440 in the {{{jQuery.fx.stop}}} function is never reached. \ \ It is also clear why this happens if you are familiar with the inner workings of jQuery for animations. The call to {{{this.options.complete.call( this.elem );}}} on line 401 in {{{jQuery.fx.step}}} throws the exception thus (further up the call stack) the code on the lines 430-435 in {{{jQuery.fx.tick()}}} won't ever be reached which means the {{{timers/jQuery.timers}}} array is never emptied and {{{jQuery.fx.stop}}} never called. \ \ I understand that it is debatable if this fix should even be applied as it masks an exception and the root fault which triggers this bug is caused by the "programmer" and not directly by jQuery. \ \ So if the decision is to not apply this fix both tickets this one and #2846 should be marked as nofix. But we should add a note to the animation documentation that a fault callback function can have this nasty endless "recursion" effect. \ \ ---- \ \ Before reporting I tested in Opera 10.10, FF 3.5.6 and IE6. \ \ Opera 10.10 and FF 3.5.6 show the reported behavior. \ \ IE6 seems to degrade in some strange way also fails to run any animations afterwards \ \ Can't test on IE7, IE8, Safari, Chrome, ...This bug was (in some sense) already discovered 1 year ago (Ticket #2846), but no action was taken.\ \ If a function, which is passed as callback function to any animation-effect-function ({{{animate}}}/{{{show}}}/{{{hide}}}/...), throws an exception jQuery generates an endless loop (actually it's a function called via a {{{setInterval}}} which is never canceled and the {{{setInterval}}} keeps running every 13ms(!).\ \ All linenumbers refer to [http://github.com/jquery/jquery/blob/1.4a2/src/effects.js effects.js jQuery 1.4a2]\ \ The proposed patch is (XXXX to be replaced by actual ticket number)\ \ {{{\ 401c401\ 401c401,402\ < this.options.complete.call( this.elem );\ ---\ > // ignore exceptions in callback check Ticket #XXXX\ > try { this.options.complete.call( this.elem ); } catch () { }\ \ }}}\ \ ----\ \ More in detail the following things can happen\ \ - the callback function is called repeatedly forever (unless at a certain time it stops throwing the exception)\ \ - if multiple elements where being animated the callback function is only called for the first element which finished animating\ \ - if the reason for the callback function to throw an exception depends on the element which finished animating, the callback is never called for all elements which complete after the first element which caused the exception in the callback\ \ - all animations started, after such a faulty callback function was triggered, fail to work completely\ \ - depending on browser + hardware CPU utilization could og to 100% or even whole browser/OS could stall\ \ - other jQuery code not related to animation continues too function normally\ \ \ As already pointed out in the other bug ticket the cause for this is that the call to {{{clearInterval( timerId );}}} on line 440 in the {{{jQuery.fx.stop}}} function is never reached.\ \ It is also clear why this happens if you are familiar with the inner workings of jQuery for animations. The call to {{{this.options.complete.call( this.elem );}}} on line 401 in {{{jQuery.fx.step}}} throws the exception thus (further up the call stack) the code on the lines 430-435 in {{{jQuery.fx.tick()}}} won't ever be reached which means the {{{timers/jQuery.timers}}} array is never emptied and {{{jQuery.fx.stop}}} never called.\ \ I understand that it is debatable if this fix should even be applied as it masks an exception and the root fault which triggers this bug is caused by the "programmer" and not directly by jQuery.\ \ So if the decision is to not apply this fix both tickets this one and #2846 should be marked as nofix. But we should add a note to the animation documentation that a fault callback function can have this nasty endless "recursion" effect.\ \ ----\ \ Before reporting I tested in Opera 10.10, FF 3.5.6 and IE6.\ \ Opera 10.10 and FF 3.5.6 show the reported behavior.\ \ IE6 seems to degrade in some strange way also fails to run any animations afterwards\ \ Can't test on IE7, IE8, Safari, Chrome, ...

+0, any perf tests on the speed impact of running through try/catch?

Changed May 23, 2011 09:19PM UTC by dmethvin comment:13

description: This bug was (in some sense) already discovered 1 year ago (Ticket #2846), but no action was taken.\ \ If a function, which is passed as callback function to any animation-effect-function ({{{animate}}}/{{{show}}}/{{{hide}}}/...), throws an exception jQuery generates an endless loop (actually it's a function called via a {{{setInterval}}} which is never canceled and the {{{setInterval}}} keeps running every 13ms(!).\ \ All linenumbers refer to [http://github.com/jquery/jquery/blob/1.4a2/src/effects.js effects.js jQuery 1.4a2]\ \ The proposed patch is (XXXX to be replaced by actual ticket number)\ \ {{{\ 401c401\ 401c401,402\ < this.options.complete.call( this.elem );\ ---\ > // ignore exceptions in callback check Ticket #XXXX\ > try { this.options.complete.call( this.elem ); } catch () { }\ \ }}}\ \ ----\ \ More in detail the following things can happen\ \ - the callback function is called repeatedly forever (unless at a certain time it stops throwing the exception)\ \ - if multiple elements where being animated the callback function is only called for the first element which finished animating\ \ - if the reason for the callback function to throw an exception depends on the element which finished animating, the callback is never called for all elements which complete after the first element which caused the exception in the callback\ \ - all animations started, after such a faulty callback function was triggered, fail to work completely\ \ - depending on browser + hardware CPU utilization could og to 100% or even whole browser/OS could stall\ \ - other jQuery code not related to animation continues too function normally\ \ \ As already pointed out in the other bug ticket the cause for this is that the call to {{{clearInterval( timerId );}}} on line 440 in the {{{jQuery.fx.stop}}} function is never reached.\ \ It is also clear why this happens if you are familiar with the inner workings of jQuery for animations. The call to {{{this.options.complete.call( this.elem );}}} on line 401 in {{{jQuery.fx.step}}} throws the exception thus (further up the call stack) the code on the lines 430-435 in {{{jQuery.fx.tick()}}} won't ever be reached which means the {{{timers/jQuery.timers}}} array is never emptied and {{{jQuery.fx.stop}}} never called.\ \ I understand that it is debatable if this fix should even be applied as it masks an exception and the root fault which triggers this bug is caused by the "programmer" and not directly by jQuery.\ \ So if the decision is to not apply this fix both tickets this one and #2846 should be marked as nofix. But we should add a note to the animation documentation that a fault callback function can have this nasty endless "recursion" effect.\ \ ----\ \ Before reporting I tested in Opera 10.10, FF 3.5.6 and IE6.\ \ Opera 10.10 and FF 3.5.6 show the reported behavior.\ \ IE6 seems to degrade in some strange way also fails to run any animations afterwards\ \ Can't test on IE7, IE8, Safari, Chrome, ...This bug was (in some sense) already discovered 1 year ago (Ticket #2846), but no action was taken. \ \ If a function, which is passed as callback function to any animation-effect-function ({{{animate}}}/{{{show}}}/{{{hide}}}/...), throws an exception jQuery generates an endless loop (actually it's a function called via a {{{setInterval}}} which is never canceled and the {{{setInterval}}} keeps running every 13ms(!). \ \ All linenumbers refer to [http://github.com/jquery/jquery/blob/1.4a2/src/effects.js effects.js jQuery 1.4a2] \ \ The proposed patch is (XXXX to be replaced by actual ticket number) \ \ {{{ \ 401c401 \ 401c401,402 \ < this.options.complete.call( this.elem ); \ --- \ > // ignore exceptions in callback check Ticket #XXXX \ > try { this.options.complete.call( this.elem ); } catch () { } \ \ }}} \ \ ---- \ \ More in detail the following things can happen \ \ - the callback function is called repeatedly forever (unless at a certain time it stops throwing the exception) \ \ - if multiple elements where being animated the callback function is only called for the first element which finished animating \ \ - if the reason for the callback function to throw an exception depends on the element which finished animating, the callback is never called for all elements which complete after the first element which caused the exception in the callback \ \ - all animations started, after such a faulty callback function was triggered, fail to work completely \ \ - depending on browser + hardware CPU utilization could og to 100% or even whole browser/OS could stall \ \ - other jQuery code not related to animation continues too function normally \ \ \ As already pointed out in the other bug ticket the cause for this is that the call to {{{clearInterval( timerId );}}} on line 440 in the {{{jQuery.fx.stop}}} function is never reached. \ \ It is also clear why this happens if you are familiar with the inner workings of jQuery for animations. The call to {{{this.options.complete.call( this.elem );}}} on line 401 in {{{jQuery.fx.step}}} throws the exception thus (further up the call stack) the code on the lines 430-435 in {{{jQuery.fx.tick()}}} won't ever be reached which means the {{{timers/jQuery.timers}}} array is never emptied and {{{jQuery.fx.stop}}} never called. \ \ I understand that it is debatable if this fix should even be applied as it masks an exception and the root fault which triggers this bug is caused by the "programmer" and not directly by jQuery. \ \ So if the decision is to not apply this fix both tickets this one and #2846 should be marked as nofix. But we should add a note to the animation documentation that a fault callback function can have this nasty endless "recursion" effect. \ \ ---- \ \ Before reporting I tested in Opera 10.10, FF 3.5.6 and IE6. \ \ Opera 10.10 and FF 3.5.6 show the reported behavior. \ \ IE6 seems to degrade in some strange way also fails to run any animations afterwards \ \ Can't test on IE7, IE8, Safari, Chrome, ...

+1, If we could use timmywil's suggestion maybe? I don't like a blind try/catch suggested by the OP.

Changed May 26, 2011 07:48AM UTC by mail@jasonwoods.me.uk comment:14

Hi all,

One doesn't need a try/catch... simply calling the completion callback AFTER dropping the animation from the timers queue means the exception occurs ONCE. I agree that a blind try/catch would hide the exception and this is NOT correct behaviour. The issue here is an exception looping. The correct behaviour would be for the exception to occur once - and try/catch would result in no exception and therefore not be correct.

I attached a patch for this for 1.4.4 which moves the callback so it is the LAST thing jQuery calls as part of the animation step and it works perfectly - it merely adds 2 extra compares to the animation and returns a callback instead of "true" from step(). This is then handled in the functions that called step() and the callback called as soon as all jQuery animation code is finished. As a result, even if the callback fails - jQuery had nothing else to do anyway so we no longer get a loop.

Just let me know if you want an updated patch for 1.6.1 or something. I'm glad to help.

Jason.

Changed May 26, 2011 07:58AM UTC by Driskell comment:15

_comment0: Just registered an account and hoping I'll get email notifications this time! :)1306396853835794

+1, Just registered an account and hoping I'll get email notifications this time! :)

(I'm the Jason mail@ guy)

Changed June 03, 2011 01:26PM UTC by john comment:16

description: This bug was (in some sense) already discovered 1 year ago (Ticket #2846), but no action was taken. \ \ If a function, which is passed as callback function to any animation-effect-function ({{{animate}}}/{{{show}}}/{{{hide}}}/...), throws an exception jQuery generates an endless loop (actually it's a function called via a {{{setInterval}}} which is never canceled and the {{{setInterval}}} keeps running every 13ms(!). \ \ All linenumbers refer to [http://github.com/jquery/jquery/blob/1.4a2/src/effects.js effects.js jQuery 1.4a2] \ \ The proposed patch is (XXXX to be replaced by actual ticket number) \ \ {{{ \ 401c401 \ 401c401,402 \ < this.options.complete.call( this.elem ); \ --- \ > // ignore exceptions in callback check Ticket #XXXX \ > try { this.options.complete.call( this.elem ); } catch () { } \ \ }}} \ \ ---- \ \ More in detail the following things can happen \ \ - the callback function is called repeatedly forever (unless at a certain time it stops throwing the exception) \ \ - if multiple elements where being animated the callback function is only called for the first element which finished animating \ \ - if the reason for the callback function to throw an exception depends on the element which finished animating, the callback is never called for all elements which complete after the first element which caused the exception in the callback \ \ - all animations started, after such a faulty callback function was triggered, fail to work completely \ \ - depending on browser + hardware CPU utilization could og to 100% or even whole browser/OS could stall \ \ - other jQuery code not related to animation continues too function normally \ \ \ As already pointed out in the other bug ticket the cause for this is that the call to {{{clearInterval( timerId );}}} on line 440 in the {{{jQuery.fx.stop}}} function is never reached. \ \ It is also clear why this happens if you are familiar with the inner workings of jQuery for animations. The call to {{{this.options.complete.call( this.elem );}}} on line 401 in {{{jQuery.fx.step}}} throws the exception thus (further up the call stack) the code on the lines 430-435 in {{{jQuery.fx.tick()}}} won't ever be reached which means the {{{timers/jQuery.timers}}} array is never emptied and {{{jQuery.fx.stop}}} never called. \ \ I understand that it is debatable if this fix should even be applied as it masks an exception and the root fault which triggers this bug is caused by the "programmer" and not directly by jQuery. \ \ So if the decision is to not apply this fix both tickets this one and #2846 should be marked as nofix. But we should add a note to the animation documentation that a fault callback function can have this nasty endless "recursion" effect. \ \ ---- \ \ Before reporting I tested in Opera 10.10, FF 3.5.6 and IE6. \ \ Opera 10.10 and FF 3.5.6 show the reported behavior. \ \ IE6 seems to degrade in some strange way also fails to run any animations afterwards \ \ Can't test on IE7, IE8, Safari, Chrome, ...This bug was (in some sense) already discovered 1 year ago (Ticket #2846), but no action was taken.\ \ If a function, which is passed as callback function to any animation-effect-function ({{{animate}}}/{{{show}}}/{{{hide}}}/...), throws an exception jQuery generates an endless loop (actually it's a function called via a {{{setInterval}}} which is never canceled and the {{{setInterval}}} keeps running every 13ms(!).\ \ All linenumbers refer to [http://github.com/jquery/jquery/blob/1.4a2/src/effects.js effects.js jQuery 1.4a2]\ \ The proposed patch is (XXXX to be replaced by actual ticket number)\ \ {{{\ 401c401\ 401c401,402\ < this.options.complete.call( this.elem );\ ---\ > // ignore exceptions in callback check Ticket #XXXX\ > try { this.options.complete.call( this.elem ); } catch () { }\ \ }}}\ \ ----\ \ More in detail the following things can happen\ \ - the callback function is called repeatedly forever (unless at a certain time it stops throwing the exception)\ \ - if multiple elements where being animated the callback function is only called for the first element which finished animating\ \ - if the reason for the callback function to throw an exception depends on the element which finished animating, the callback is never called for all elements which complete after the first element which caused the exception in the callback\ \ - all animations started, after such a faulty callback function was triggered, fail to work completely\ \ - depending on browser + hardware CPU utilization could og to 100% or even whole browser/OS could stall\ \ - other jQuery code not related to animation continues too function normally\ \ \ As already pointed out in the other bug ticket the cause for this is that the call to {{{clearInterval( timerId );}}} on line 440 in the {{{jQuery.fx.stop}}} function is never reached.\ \ It is also clear why this happens if you are familiar with the inner workings of jQuery for animations. The call to {{{this.options.complete.call( this.elem );}}} on line 401 in {{{jQuery.fx.step}}} throws the exception thus (further up the call stack) the code on the lines 430-435 in {{{jQuery.fx.tick()}}} won't ever be reached which means the {{{timers/jQuery.timers}}} array is never emptied and {{{jQuery.fx.stop}}} never called.\ \ I understand that it is debatable if this fix should even be applied as it masks an exception and the root fault which triggers this bug is caused by the "programmer" and not directly by jQuery.\ \ So if the decision is to not apply this fix both tickets this one and #2846 should be marked as nofix. But we should add a note to the animation documentation that a fault callback function can have this nasty endless "recursion" effect.\ \ ----\ \ Before reporting I tested in Opera 10.10, FF 3.5.6 and IE6.\ \ Opera 10.10 and FF 3.5.6 show the reported behavior.\ \ IE6 seems to degrade in some strange way also fails to run any animations afterwards\ \ Can't test on IE7, IE8, Safari, Chrome, ...

+1, This seems like something that we could work around with just some minor logic tweaking (we certainly don't want to hide the execeptions, though).

Changed June 03, 2011 02:23PM UTC by scottgonzalez comment:17

+1, repeating errors is obviously wrong

Changed June 06, 2011 03:39PM UTC by jzaefferer comment:18

+0

Changed June 06, 2011 03:51PM UTC by cowboy comment:19

+0

Changed July 11, 2011 09:17PM UTC by ajpiano comment:20

description: This bug was (in some sense) already discovered 1 year ago (Ticket #2846), but no action was taken.\ \ If a function, which is passed as callback function to any animation-effect-function ({{{animate}}}/{{{show}}}/{{{hide}}}/...), throws an exception jQuery generates an endless loop (actually it's a function called via a {{{setInterval}}} which is never canceled and the {{{setInterval}}} keeps running every 13ms(!).\ \ All linenumbers refer to [http://github.com/jquery/jquery/blob/1.4a2/src/effects.js effects.js jQuery 1.4a2]\ \ The proposed patch is (XXXX to be replaced by actual ticket number)\ \ {{{\ 401c401\ 401c401,402\ < this.options.complete.call( this.elem );\ ---\ > // ignore exceptions in callback check Ticket #XXXX\ > try { this.options.complete.call( this.elem ); } catch () { }\ \ }}}\ \ ----\ \ More in detail the following things can happen\ \ - the callback function is called repeatedly forever (unless at a certain time it stops throwing the exception)\ \ - if multiple elements where being animated the callback function is only called for the first element which finished animating\ \ - if the reason for the callback function to throw an exception depends on the element which finished animating, the callback is never called for all elements which complete after the first element which caused the exception in the callback\ \ - all animations started, after such a faulty callback function was triggered, fail to work completely\ \ - depending on browser + hardware CPU utilization could og to 100% or even whole browser/OS could stall\ \ - other jQuery code not related to animation continues too function normally\ \ \ As already pointed out in the other bug ticket the cause for this is that the call to {{{clearInterval( timerId );}}} on line 440 in the {{{jQuery.fx.stop}}} function is never reached.\ \ It is also clear why this happens if you are familiar with the inner workings of jQuery for animations. The call to {{{this.options.complete.call( this.elem );}}} on line 401 in {{{jQuery.fx.step}}} throws the exception thus (further up the call stack) the code on the lines 430-435 in {{{jQuery.fx.tick()}}} won't ever be reached which means the {{{timers/jQuery.timers}}} array is never emptied and {{{jQuery.fx.stop}}} never called.\ \ I understand that it is debatable if this fix should even be applied as it masks an exception and the root fault which triggers this bug is caused by the "programmer" and not directly by jQuery.\ \ So if the decision is to not apply this fix both tickets this one and #2846 should be marked as nofix. But we should add a note to the animation documentation that a fault callback function can have this nasty endless "recursion" effect.\ \ ----\ \ Before reporting I tested in Opera 10.10, FF 3.5.6 and IE6.\ \ Opera 10.10 and FF 3.5.6 show the reported behavior.\ \ IE6 seems to degrade in some strange way also fails to run any animations afterwards\ \ Can't test on IE7, IE8, Safari, Chrome, ...This bug was (in some sense) already discovered 1 year ago (Ticket #2846), but no action was taken. \ \ If a function, which is passed as callback function to any animation-effect-function ({{{animate}}}/{{{show}}}/{{{hide}}}/...), throws an exception jQuery generates an endless loop (actually it's a function called via a {{{setInterval}}} which is never canceled and the {{{setInterval}}} keeps running every 13ms(!). \ \ All linenumbers refer to [http://github.com/jquery/jquery/blob/1.4a2/src/effects.js effects.js jQuery 1.4a2] \ \ The proposed patch is (XXXX to be replaced by actual ticket number) \ \ {{{ \ 401c401 \ 401c401,402 \ < this.options.complete.call( this.elem ); \ --- \ > // ignore exceptions in callback check Ticket #XXXX \ > try { this.options.complete.call( this.elem ); } catch () { } \ \ }}} \ \ ---- \ \ More in detail the following things can happen \ \ - the callback function is called repeatedly forever (unless at a certain time it stops throwing the exception) \ \ - if multiple elements where being animated the callback function is only called for the first element which finished animating \ \ - if the reason for the callback function to throw an exception depends on the element which finished animating, the callback is never called for all elements which complete after the first element which caused the exception in the callback \ \ - all animations started, after such a faulty callback function was triggered, fail to work completely \ \ - depending on browser + hardware CPU utilization could og to 100% or even whole browser/OS could stall \ \ - other jQuery code not related to animation continues too function normally \ \ \ As already pointed out in the other bug ticket the cause for this is that the call to {{{clearInterval( timerId );}}} on line 440 in the {{{jQuery.fx.stop}}} function is never reached. \ \ It is also clear why this happens if you are familiar with the inner workings of jQuery for animations. The call to {{{this.options.complete.call( this.elem );}}} on line 401 in {{{jQuery.fx.step}}} throws the exception thus (further up the call stack) the code on the lines 430-435 in {{{jQuery.fx.tick()}}} won't ever be reached which means the {{{timers/jQuery.timers}}} array is never emptied and {{{jQuery.fx.stop}}} never called. \ \ I understand that it is debatable if this fix should even be applied as it masks an exception and the root fault which triggers this bug is caused by the "programmer" and not directly by jQuery. \ \ So if the decision is to not apply this fix both tickets this one and #2846 should be marked as nofix. But we should add a note to the animation documentation that a fault callback function can have this nasty endless "recursion" effect. \ \ ---- \ \ Before reporting I tested in Opera 10.10, FF 3.5.6 and IE6. \ \ Opera 10.10 and FF 3.5.6 show the reported behavior. \ \ IE6 seems to degrade in some strange way also fails to run any animations afterwards \ \ Can't test on IE7, IE8, Safari, Chrome, ...
milestone: 1.next1.7

Targeting for 1.7 at bug triage

Changed August 15, 2011 10:40PM UTC by gnarf comment:21

owner: → gnarf
status: openassigned

I'm gonna pick this one up and see if I can do anything with it.

Changed August 25, 2011 05:01AM UTC by paulpro@uvic.ca comment:22

Rather than absorb the error, we could execute the callback asynchronously, so that it will still throw the error, but won't affect the current running thread. It's a very simple edit to make. Can anyone think of any reason not to do it like this:

setTimeout(function(){
	options.complete.call(elem);
}, 1);

Changed September 27, 2011 08:54AM UTC by gnarf comment:23

Changed September 28, 2011 04:01PM UTC by Corey Frang comment:24

resolution: → fixed
status: assignedclosed

Landing pull request 520. Unset the complete function just before calling it to avoid an exception creating a loop. Fixes #5684.

More Details:

Changeset: 8dda57f82f1c366f10c591da099f8e9495eaa01b

Changed October 05, 2011 08:49PM UTC by timmywil comment:25

#10425 is a duplicate of this ticket.