Bug P2
Status Update
Comments
mj...@google.com <mj...@google.com>
is...@google.com <is...@google.com>
ni...@gmail.com <ni...@gmail.com> #2
P4
ng...@gmail.com <ng...@gmail.com> #3
ng...@gmail.com <ng...@gmail.com> #4
yu...@google.com <yu...@google.com>
jr...@minervanetworks.com <jr...@minervanetworks.com> #5
Any update?
Description
Although `analytics.js` is minified, the code itself is easy to understand. The `keys` array is a property of a two-tiered map. It is supposed to store the combined keys of the two tiers, facilitating a `map` helper function. The code never checks if it is adding duplicate keys to the array, which is the source of the leak.
Here is the pretty printed code, with offending line annotated:
```
var ee = function() {
this.keys = [];
this.values = {};
this.m = {}
};
ee.prototype.set = function(a, b, c) {
this.keys.push(a); // <-- this should check if the key is already in the array
c ? this.m[":" + a] = b : this.values[":" + a] = b
}
;
ee.prototype.get = function(a) {
return this.m.hasOwnProperty(":" + a) ? this.m[":" + a] : this.values[":" + a]
}
;
ee.prototype.map = function(a) {
for (var b = 0; b < this.keys.length; b++) {
var c = this.keys[b]
, d = this.get(c);
d && a(c, d)
}
}
;
```
As a bonus bug, the duplicate keys mean that the `map` function will actually map over some elements twice. So this memory leak is also a correctness bug.
The following fix works, but has bad asymptotic behavior as it makes `set` O(keys):
```
ee.prototype.set = function(a, b, c) {
if (this.keys.indexOf(a) !== -1) {
this.keys.push(a);
}
c ? this.m[":" + a] = b : this.values[":" + a] = b
}
```
If the objects are used exclusively in the map (as they appear to be), a more efficient version might be:
```
ee.prototype.set = function(a, b, c) {
var innerKey = ":" + a;
// could make this into a `has()` function...
if (!this.m.hasOwnProperty(innerKey) && !this.values.hasOwnProperty(innerKey)) {
this.keys.push(a);
}
c ? this.m[innerKey] = b : this.values[innerKey] = b
}
```