Issue 2028 in v8: garbage collection issue

[codesite...@google.com]

Status: New
Owner: ----

New issue 2028 by franky.z...@gmail.com: garbage collection issue
http://code.google.com/p/v8/issues/detail?id=2028

// Suggest you copy this page into a JS editor to read.
// @Part1, This part is the API implementation code:
require.define('apis', function (require, module, exports) {
var _history = db.retriveObject(), //db is defined at other place
_listener = {},
_handler,
_self;

_self = {
find: function (successCB, filter) {
var filterResults = _history; // key place! It will work well
if I use utils.copy(_history);
if (filter) {
filterResults = filter(filterResults);
}

setTimeout(successCB(filterResults), 1);
},
addListener: function (observerObj) {
_handler = Math.uuid(null, 16);

_listener[handle] = observerObj;
}
};

event.on("newItems", function (items) {
var observerObj = _listener[_handler];

_history.push(items);
observerObj(items);
});

return _self;
});

// @Part2, The demo code:
var _historyList;

function successCB(results) {
_historyList = results;
}

function addedCB(newItems) {
console.log(_historyList); // New items have been added into
_historyList already here!
_historyList = _historyList.concat(newItems);
}
apis.find(successCB, filter);
apis.addListener(addedCB);

// Problem description:
// In the addedCB function, when it's called for the fist time,
// the newItems will be in the _historyList before it's added! But after
that, when the addedCB
// is called for the second time, no such thing happens.

// Some findings that related:
// After some debugging, I found that the "_historyList" variable in demo
application has pointed
// to the "_history" in apis module for the first time the addedCB is
called. There is a connection
// between "_historyList" in demo and "_history" in the API module.

// The reason and problem that I guess:
// I guess that "filterResults" will be cleared automatically when the
programme exit from "find"
// function, but as it points to "_history", it won't be cleared
immediately, it will be cleared
// automatically only when the "_history" changes.
// The problem is here: When the "_history" changes, all the local
variables that point to
// it should be cleared in the memory, but this should be done
before "_history" changes, not after,
// they may mistake the order so that before those variables are cleared,
they got the new values and
// those values are passed to any variable that points to it. For the
second time it's called, the
// connection has been broke so the "_historyList" in the demo won't get
the latest value of "_history"
// in API module.

// As I am not familiar with the source code of V8, so the location of the
problem has not been found in
// the source code. Here I just report it and expect some expert to confirm
it.

[codesite...@google.com]

Updates:
Status: WorkingAsIntended
Owner: mstar...@chromium.org

Comment #1 on issue 2028 by mstar...@chromium.org: garbage collection
issue
http://code.google.com/p/v8/issues/detail?id=2028

I don't see the garbage collection related problem in your snippet. When
successCB() is called, it will set _historyList either to _history or
whatever the filter() method will return. So it might end up pointing to
the same object the db.retrieveObject() returns.

The concat() builtin in addedCB() however will always give you a new Array
object containing the concatenation of the old _historyList and newItems.

I am not sure what you mean by "filterResults will be cleared
automatically". It is a local variable that will cease to exist when it
runs out of scope. The _histroyList variable however is a global variable
that will exist throughout the runtime of your application. The GC will
never modify it's content (at least not in a way that is visible to you).

[codesite...@google.com]

Comment #2 on issue 2028 by franky.z...@gmail.com: garbage collection issue
http://code.google.com/p/v8/issues/detail?id=2028

The variable apis will always exist when the demo application is running,
so the "_history" will always there.
When successCB() is called, "_historyList" is set to "_history" and in the
event.on("newItems", function (items) {}); the connection between them is
still available until "_history.push(items);" is implemented.
So before observerObj(items); the "_historyList" has been updated with the
new items, that will be shown when the value of "_historyList" is printed.

[codesite...@google.com]

Comment #3 on issue 2028 by franky.z...@gmail.com: garbage collection issue
http://code.google.com/p/v8/issues/detail?id=2028

// Sample code here, expect this will express the problem clearly
function API() {
var _history = ["Base Item"],

_listener = {},
_handler,
_self;

this.find = function (successCB, filter) {

var filterResults = _history; // key place! It will work well if I
use utils.copy(_history);
if (filter) {
filterResults = filter(filterResults);
}

setTimeout(successCB(filterResults), 1);
};

this.addListener = function (observerObj) {
_handler = (new Date()).getTime();

_listener[_handler] = observerObj;
};

this.trigger = function (newItems) {

var observerObj = _listener[_handler];

_history.push(newItems);
observerObj(newItems);
};
}

function demo(apis) {
var _historyList;

function successCB(results) {
_historyList = results;
}

function addedCB(newItems) {
// What should be printed here? [] ? ["Base Item"] ? ["Base
Item", "New Item 1"]?
console.log(_historyList);
_historyList = _historyList.concat(newItems);
}
apis.find(successCB, null);
apis.addListener(addedCB);

apis.trigger("New Item 1");
//apis.trigger(["New Item 2"]);
//apis.trigger(["New Item 3"]);
}

var apis = new API();
demo(apis);

[codesite...@google.com]

Comment #4 on issue 2028 by franky.z...@gmail.com: garbage collection issue
http://code.google.com/p/v8/issues/detail?id=2028

It will work as I expect if I exchange the order of these two lines:

_history.push(newItems);
observerObj(newItems);

to be:

observerObj(newItems);
_history.push(newItems);

[codesite...@google.com]

Comment #5 on issue 2028 by jkum...@chromium.org: garbage collection issue
http://code.google.com/p/v8/issues/detail?id=2028

Sounds like you should read up on your understanding of variables and the
way they reference objects in memory. Do you know what a "pointer" is?

When your code executes, _history and _historyList will initially refer to
the same array in memory. If you keep simply keep it that way, you don't
need your addedCB callback at all. (You also break any abstraction you
intended to have.)
Array.push modifies the receiver in place. So after _history.push(newItems)
is executed, _historyList still points to the same array object in memory,
which has now been modified.
Array.concat, however, creates a new object that consists of copies of the
parameters to the function. So after _historyList =
_historyList.concat(newItems) is executed, _historyList will point to a new
object in memory. Therefore, when _history.push() is executed again, the
object pointed to by _historyList will not be affected.

This really has nothing to do with garbage collection -- in fact, it has
nothing to do with V8 or even Javascript, you could encounter the same
situation in C++ or Java or Python or pretty much any language of your
choice.

[codesite...@google.com]

Comment #6 on issue 2028 by franky.z...@gmail.com: garbage collection issue
http://code.google.com/p/v8/issues/detail?id=2028

jkum...@chromium.org, thank you for your comments, I get it now.