[Pkg-javascript-commits] [dojo] 81/88: Clear references within dojo/aspect when handles are removed to avoid over-retention of objects when a user removes a handle and then does not dereference the handle. Fixes #17048.

[David Prévot]

This is an automated email from the git hooks/post-receive script.

taffit pushed a commit to annotated tag 1.8.5
in repository dojo.

commit 3d25d1e5b0831aabce2b7ad06bb1d7d2ffe1fad1
Author: Colin Snover <github.com at zetafleet.com>
Date:   Thu Apr 18 17:49:10 2013 +0000

    Clear references within dojo/aspect when handles are removed to avoid
    over-retention of objects when a user removes a handle and then does
    not dereference the handle. Fixes #17048.
    
    Prevent multiple remove() calls on an aspect signal from corrupting the aspect chain, fixes #16513
    
    !strict due to "use strict" pragma
    
    Backport to 1.8
    
    
    git-svn-id: http://svn.dojotoolkit.org/src/branches/1.8/dojo@31299 560b804f-0ae3-0310-86f3-f6aa0a117693
---
 aspect.js       | 40 ++++++++++++++++++++++++----------------
 tests/aspect.js | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 16 deletions(-)

diff --git a/aspect.js b/aspect.js
index 8f9612c..1f19291 100644
--- a/aspect.js
+++ b/aspect.js
@@ -15,31 +15,39 @@ define([], function(){
 			});
 			signal = {
 				remove: function(){
-					signal.cancelled = true;
+					if(advised){
+						advised.remove();
+						advised = dispatcher = advice = null;
+					}
 				},
 				advice: function(target, args){
-					return signal.cancelled ?
-						previous.advice(target, args) : // cancelled, skip to next one
-						advised.apply(target, args);	// called the advised function
+					return advised ?
+						advised.apply(target, args) :  // called the advised function
+						previous.advice(target, args); // cancelled, skip to next one
 				}
 			};
 		}else{
 			// create the remove handler
 			signal = {
 				remove: function(){
-					var previous = signal.previous;
-					var next = signal.next;
-					if(!next && !previous){
-						delete dispatcher[type];
-					}else{
-						if(previous){
-							previous.next = next;
+					if(signal.advice){
+						var previous = signal.previous;
+						var next = signal.next;
+						if(!next && !previous){
+							delete dispatcher[type];
 						}else{
-							dispatcher[type] = next;
-						}
-						if(next){
-							next.previous = previous;
+							if(previous){
+								previous.next = next;
+							}else{
+								dispatcher[type] = next;
+							}
+							if(next){
+								next.previous = previous;
+							}
 						}
+
+						// remove the advice to signal that this signal has been removed
+						dispatcher = advice = signal.advice = null;
 					}
 				},
 				id: nextId++,
@@ -50,7 +58,7 @@ define([], function(){
 		if(previous && !around){
 			if(type == "after"){
 				// add the listener to the end of the list
-				// note that we had to change this loop a little bit to workaround a bizarre IE10 JIT bug 
+				// note that we had to change this loop a little bit to workaround a bizarre IE10 JIT bug
 				while(previous.next && (previous = previous.next)){}
 				previous.next = signal;
 				signal.previous = previous;
diff --git a/tests/aspect.js b/tests/aspect.js
index 1d9b582..9f50f04 100644
--- a/tests/aspect.js
+++ b/tests/aspect.js
@@ -82,6 +82,17 @@ doh.register("tests.aspect",
 			}, true);
 			t.t(obj.method() === false);
 		},
+		function afterMultiple(t){
+			var order = [];
+			obj = {
+				foo: function(){}
+			};
+			aspect.after(obj, "foo", function(){order.push(1)});
+			aspect.after(obj, "foo", function(){order.push(2)});
+			aspect.after(obj, "foo", function(){order.push(3)});
+			obj.foo();
+			t.is([1,2,3], order);
+		},
 		function around(t){
 			var order = [];
 			var obj = {
@@ -105,6 +116,36 @@ doh.register("tests.aspect",
 			obj.method(4);
 			t.is(order, [0,1,2,3,4,5,6]);
 		},
+		function multipleRemove(t){
+			var foo = {bar: function(){}};
+			var order = [];
+			var signal1 = aspect.after(foo, "bar", function() {
+	    		order.push(1);
+			});
+
+			var signal2 = aspect.after(foo, "bar", function() {
+				order.push(2);
+			});
+
+			var signal3 = aspect.after(foo, "bar", function() {
+				order.push(3);
+			});
+
+			// This should execute all 3 callbacks
+			foo.bar();
+			
+			signal2.remove();
+			signal3.remove();
+
+			// Ideally signal2 should not be removed again, but can happen if the app
+			// fails to clear its state.
+			signal2.remove();
+			
+			// This should execute only the first callback, but notice that the third callback
+			// is executed as well
+			foo.bar();
+			t.is(order, [1,2,3,1]);
+		},
 		function delegation(t){
 			var order = [];
 			var proto = {

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-javascript/dojo.git