[Ur] Substantial memory leak in the JavaScript runtime

[Saulo Araujo]

Hi,

Did anyone had the opportunity to look into this bug report and in the
proposed fix?

Sincerely,
Saulo

On Fri, Aug 12, 2016 at 10:55 PM, Saulo Araujo <saulo2 at gmail.com> wrote:

> Hi,
>
> I believe there is a substantial memory leak in the JavaScript runtime of
> Ur/Web. To see it happening:
>
> 1) visit http://timesheet-ur.sauloaraujo.com:8080/TimeSheet/application
> with Google Chrome
> 2) open the developer tools by pressing f12
> 3) click in the "Profiles" tab
> 4) click in the "Take Heap Snapshot" radio button
> 5) click in the "Take Snapshot" button
> 6) type Detached in the "Class filter" text box (you will see that the
> heap has about 7MB and Detached DOM trees are retaining 0% of the memory -
> https://snag.gy/2nF9fz.jpg)
>
> 7) click 10 times in the foward icon (the one on the right of the "Date"
> header in the time sheet application)
> 9) click in the "Take Snapshot" button
> 10) type Detached in the "Class filter" text box (you will see that the
> heap has about 43MB and Detached DOM trees are retaining 16% of the memory
> - https://snag.gy/Crc3Vg.jpg)
>
> 11) click 10 times in the foward icon (the one on the right of the "Date"
> header in the time sheet application)
> 12) click in the "Take Snapshot" button
> 13) type Detached in the "Class filter" text box (you will see that the
> heap has about 78MB and Detached DOM trees are retaining 17% of the memory
> - https://snag.gy/P5BLhq.jpg)
>
> I have spent quite some time investigating this memory leak and I believe
> the problem is in the function recreate inside the function dyn:
>
> function dyn(pnode, s) {
>     if (suspendScripts)
>         return;
>
>     var x = document.createElement("script");
>     x.dead = false;
>     x.signal = s;
>     x.sources = null;
>     x.closures = null;
>
>     var firstChild = null;
>
>     x.recreate = function(v) {
>         for (var ls = x.closures; ls; ls = ls.next)
>             freeClosure(ls.data);
>
>         var next;
>         for (var child = firstChild; child && child != x; child = next) {
>             next = child.nextSibling;
>
>             killScript(child);
>             if (child.getElementsByTagName) {
>                 var arr = child.getElementsByTagName("script");
>                 for (var i = 0; i < arr.length; ++i)
>                     killScript(arr[i]);
>             }
> ...
>
> Note that recreate only kills <script> nodes . Therefore, <input>,
> <textarea> and <select> nodes created through <ctextbox>, <ctextarea> and
> <cselect> will continue to be in the dyns lists of its sources. To fix this
> memory leak, I propose changing the function dyn to
>
> function dyn(pnode, s) {
>     if (suspendScripts)
>         return;
>
>     var x = document.createElement("script");
>     x.dead = false;
>     x.signal = s;
>     x.sources = null;
>     x.closures = null;
>
>     var firstChild = null;
>
>     x.recreate = function(v) {
>         for (var ls = x.closures; ls; ls = ls.next)
>             freeClosure(ls.data);
>
>         var next;
>         for (var child = firstChild; child && child != x; child = next) {
>             next = child.nextSibling;
>
>             killDyns(child)
> ...
>
> Below is the function killDyns.
>
> function killDyns(node) {
>     var nodeIterator = document.createNodeIterator(node,
> NodeFilter.SHOW_ELEMENT, function (node) {
>         return node.dead !== undefined && node.dead === false
>     })
>     var node = nodeIterator.nextNode();
>     while (node) {
>         killScript(node);
>         node = nodeIterator.nextNode();
>     }
> }
>
> killDyns traverses all descendants of a node that have a dead attribute
> equal to false and kills them with killScript. Therefore, killDyns may be
> less performant than the code it substitutes, but I believe killDyns is
> less prone to memory leaks than the original code. There is another small
> change to be done in urweb.js. You can see all changes in
> https://github.com/saulo2/urweb/commit/dcd280c85595ceee60a4fb78a3bfaf
> 413a9eb7b8#diff-867e8dfbbc36419eefc0dfdf9db32883
>
> I did not make a pull request yet because I would like to know the opinion
> of the Ur/Web comitters about this fix. Maybe there is something that can
> be improved. Maybe there is a new bug in it :) In any case, repeating the
> previous steps with the proposed changes, I get the following results:
>
> 6) type Detached in the "Class filter" text box (you will see that the
> heap has about 7MB and Detached DOM trees are retaining 0% of the memory -
> https://snag.gy/NicJow.jpg)
> 10) type Detached in the "Class filter" text box (you will see that the
> heap has about 15MB and Detached DOM trees are retaining 0% of the memory -
> https://snag.gy/nuVfhg.jpg)
> 13) type Detached in the "Class filter" text box (you will see that the
> heap has about 21MB and Detached DOM trees are retaining 0% of the memory -
> https://snag.gy/aPeUuK.jpg)
>
> The new results suggest that there is another memory leak in the
> JavaScript Runtime. However, I have not looked into it yet.
>
> Sincerely,
> Saulo
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.impredicative.com/pipermail/ur/attachments/20160820/77db3fcf/attachment.html>