λ + concurrency = evil
I ran into an awful bug in some JavaScript the other day that I thought I’d share as one more reason that non-functional programming languages are really awful in concurrent or asynchronous execution environments. Without further ado, here’s the code:
function update_watchlist() {
var selected_instruments = $('div#instruments').data( 'selected_instruments');
for( var selected = 0; selected < selected_instruments.length; ++selected ) {
$.post( '/getquote',
selected_instruments[selected],
function( quote ) {
$('div#quote_last' + selected ).html( quote.last );
$('div#quote_volume' + selected).html( quote.volume );
$('div#quote_datetime' + selected).html( quote.datetime );
},
'json'
);
}
}
This code is supposed to use JQuery and ajax to update a watch list of quotes. Data values for each quote are stored in numbered divs where the div number matches the index in selected_instruments which is an array of information needed to look up a particular quote. In other words we update div id=’quote_last0′ with a quote that we look up using information in selected_instruments[0]. Can you spot the problem?
What is the value of selected in the lambda function passed to $.post? We don’t really know because post is an asynchronous call, the lamda function
function( quote ) {
$('div#quote_last' + selected ).html( quote.last );
$('div#quote_volume' + selected).html( quote.volume );
$('div#quote_datetime' + selected).html( quote.datetime );
},
is only called after the http post to the remote server returns. By the time this has occurred, selected has passed out of scope. The fix for this problem would have been to make lambda functions in JavaScript more like first class functions so in this case, the only way to pass the index data to the lambda function illustrated above would be to pass it in the post data, and then have the server return it in the argument passed in the lambda function as illustrated below.
function update_watchlist() {
var selected_instruments = $('div#instruments').data( 'selected_instruments');
for( var selected = 0; selected < selected_instruments.length; ++selected ) {
$.post( '/getquote',
{
// selected is a valid variable here
instrument : selected_instruments[selected],
index : selected
},
function( result ) {
// result.index = selected passed to post target and returned
$('div#quote_last' + result.index ).html( result.quote.last );
$('div#quote_volume' + result.index).html( result.quote.volume );
$('div#quote_datetime' + result.index).html( result.quote.datetime );
},
'json'
);
}
}
You need to create a bunch of instances of “selected” (one for each closure) but this code only creates one and mutates it. The problem isn’t that “selected” goes out of scope, but that it stays in scope for the duration of the loop. You’re right, it changes on each iteration so its value varies. But the important thing is that there is only one “selected”. To create multiple instances you need a closure that goes out of scope on each iteration rather than at the end of the loop. In other words, you are creating “selected_instruments.length” closures but they all share the same “selected”.
A for loop does not create a new scope in javascript. To create a new scope for each iteration you can define and execute a lambda inside the loop body. I wrote two functions, a() and b(). The first one recycles x’s binding as it makes closures to print x. This is mimics the bug. The second makes a copy of x into y, which is a new binding on each iteration. a() prints all “3″‘s because that’s the last value of x. b() prints 1 through 3 like you would expect since y traps the current value of x. I tested this in spidermonkey at the console.
// the x can be anything function
function a () {
var closures = Array();
for (var x=0; x<3; x++) {
closures.push(function () {
print(x);
});
}
return closures;
}
function b () {
var closures = Array();
for (var x=0; x<3; x++) {
// create a new environment for each loop iteration
(function () {
// y will go out of scope at the end of an iteration so its value wont change in the future
var y = x;
closures.push(function () {
print(y);
});
})();
}
return closures;
}
print("a()");
var closures = a();
for (var i=0; i<3; i++) {
closures[i]();
}
print("b()");
var closures = b();
for (var i=0; i<3; i++) {
closures[i]();
}
You could use the trick in b() rather than roundtripping the index to the server.
Comment by Sam Danielson — September 25, 2010 @ 11:21 pm
Oops. Code is correct but I described the output wrong. The output looks like this.
$ js closure.js
a()
3
3
3
b()
0
1
2
Comment by Sam Danielson — September 25, 2010 @ 11:24 pm
Oops. Code is correct but I described the output wrong. Here’s the output.
$ js closure.js
a()
3
3
3
b()
0
1
2
Comment by Sam Danielson — September 25, 2010 @ 11:25 pm
In my example the lambda is a callback function supplied to an asynchronous ajax call. In my example .post gets called, sends an http POST to a remote server which then returns to the caller. It is at this time that the caller invokes the callback. Here is what I get, still have the same problem.
Rhino 1.7 release 2 2009 03 22
js> var cl = new Array();
js> for( var i = 0; i < 3; ++i ) {
> cl.push( function() {
> var x = i;
> print( “Value of i => “, x );
> });
> }
3
js> gc();
js> cl[0]();
Value of i => 3
js> cl[1]();
Value of i => 3
js> cl[2]();
Value of i => 3
js>
Comment by admin — September 26, 2010 @ 10:19 am
The “var x” binding in comment 2 is created each time the lambda is invoked. Every invocation then reads the current value of “i”, which was modified by the loop. “i” is still a shared variable and that is causing synchronization issues. Instead, create a new “var x” and destroy its binding on each iteration. Then every lambda will have it’s own “x” tucked away with no need to read a shared variable when it is invoked.
Like so….
js> var cl = new Array();
js> for (var i = 0; i “, x);
});
})(); // Close off the anonymous function to destroy x’s scope. Then execute it so cl.push(…) happens.
}
js> gc();
js> cl[0]();
Value of i => 0
js> cl[1]();
Value of i => 1
js> cl[2]();
Value of i => 2
Happy programming.
Comment by Sam Danielson — September 27, 2010 @ 10:58 am
Hehe, I am having synchronization issues. Comment 2 is now comment 4.
Comment by Sam Danielson — September 27, 2010 @ 11:00 am
Okay… not sure what’s happening. The comment box seems to have deleted the important section. Could be my current graphics driver issue where ghost text appears after I delete it and crap like that. Here’s the important section I meant to communicate.
js> for (var i = 0; i “, x);
});
})(); // Close off the anonymous function to destroy x’s scope. Then execute it so cl.push(…) happens
}
scroll scroll, switch workspace, scroll scroll, cross fingers, submit
Comment by Sam Danielson — September 27, 2010 @ 11:08 am
Sorry, man. I’ll try it without comments.
for (var i = 0; i “, x);
});
})();
}
Comment by Sam Danielson — September 27, 2010 @ 11:09 am
for (var i = 0; i “, x);
});
})();
}
Comment by Sam Danielson — September 27, 2010 @ 11:11 am
js> for (var i = 0; i (function () {
> var x = i;
> cl.push(function() {
> print(“Value of i => “, x);
> });
> })();
> }
Comment by Sam Danielson — September 27, 2010 @ 11:12 am
John, what is the trick to format code in your blog? Sorry for making such mess.
Comment by Sam Danielson — September 27, 2010 @ 11:13 am
I use this plugin http://coffee2code.com/wp-plugins/preserve-code-formatting/
Comment by admin — September 28, 2010 @ 9:19 pm