New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 692074 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Memory leak when resizing a PPAPI 3D graphics plugin

Reported by scheuerm...@barco.com, Feb 14 2017

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/55.0.2883.87 Chrome/55.0.2883.87 Safari/537.36

Steps to reproduce the problem:
1. extract the tar container and put the files on a web server
2. Load the index file
3. Start the Scaling

What is the expected behavior?
To run forever

What went wrong?
The browser eats up memory till I get "Aw, Snap!"

Did this work before? Yes 47.0.2526.106

Does this work in other browsers? Yes

Chrome version: 56.0.2924.76  Channel: stable
OS Version: Debian 8
Flash Version: 

This graphics_3d is the example from the nacl SDK examples/api/graphics_3d.
I've extended index.html with the scaling test which just resizes the embed element. I've added the following code:

  <div>
    <script type="text/javascript">
      var do_scale = false;
      var scale_width_min = 40;
      var scale_height_min = 40;
      var scale_width_max = 1000;
      var scale_height_max = 1000;
      var scale_width = 200;
      var scale_height = 200;
      var scale_step = 5;

      function scale_test() {
        var el = document.getElementById("nacl_module");
        if(!do_scale) {
          return;
        }
        scale_width = scale_width + scale_step;
        scale_height = scale_height + scale_step;
        if(scale_width >= scale_width_max || scale_height >= scale_height_max) {
          scale_step = -5;
        }
        if(scale_width <= scale_width_min || scale_height <= scale_height_min) {
          scale_step = 5;
        }
        el.style.width = scale_width + "px";
        el.style.height = scale_height + "px";
        setTimeout(scale_test, 50);
      }

      function start_scale() {
        do_scale = true;
        scale_test();
      }

      function stop_scale() {
        do_scale = false;
      }
    </script>
    <span class="label">Scaling</span>
    <label>
      off
      <input type="radio" name="scale_test" id="scalingOff" value="true" checked="true" onclick="stop_scale()">
    </label>
    <label>
      on
      <input type="radio" name="scale_test" id="scalingOn" value="false" onclick="start_scale()">
    </label>
  </div>

 
graphics_3d.tar.gz
1.2 MB Download
The problem is also visible on Windows.
Components: Internals>Plugins>Pepper Internals>GPU
Labels: -Hotlist-Interop
> Does this work in other browsers? Yes
I doubt it, since only Chromium based browsers implement PPAPI and native client. What other browsers have you tested?
This is only a chrome/chromium problem.
It worked with older versions of chrome.
I've tested it with 47.0.2526.106

Comment 5 by ajha@chromium.org, Feb 20 2017

Labels: Needs-Bisect Needs-Triage-M56
Labels: -Needs-Bisect -Needs-Triage-M56 TE-NeedsTriageHelp
This issue seems to be out of TE-scope. Hence, marking label TE-NeedsTriageHelp for further investigation.

Thanks...!!
Hi,
is there anything new on this ticket or are there any additional information needed?
The fix of this problem is very urgent for us.
The problem is still there with google chrome 51.0.2704.106-1

Comment 9 by danakj@chromium.org, Mar 22 2017

Labels: Needs-Feedback
Can you take traces (at different points in time) with memory-infra to track down where the memory leak is happening?

Instructions: https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/README.md
Owner: raymes@chromium.org
Status: Assigned (was: Unconfirmed)
Assigning to raymes to follow up once we have tracing (or maybe can help with tracing).
Cc: raymes@chromium.org
Owner: piman@chromium.org
I'm not very familiar with the 3d graphics stack for ppapi. Maybe piman@ can be of more help?

Comment 12 by piman@chromium.org, Mar 28 2017

Cc: erikc...@chromium.org
Thanks for the report. There is a very large leak of GPU memory when resizing the back buffer. The fix for that is simple (see https://codereview.chromium.org/2779873002/)

Comment 13 by piman@chromium.org, Mar 28 2017

Labels: OS-Chrome OS-Mac OS-Windows
I should note that this is not restricted to NaCl, but would affect Flash as well. We may want to consider a merge.

Comment 14 by piman@chromium.org, Mar 28 2017

Components: -Internals>GPU Internals>GPU>Internals
Labels: -Pri-2 M-58 Pri-1
Project Member

Comment 15 by bugdroid1@chromium.org, Mar 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/15176a8d134862de21fe2de9f9fd1d7390ac9003

commit 15176a8d134862de21fe2de9f9fd1d7390ac9003
Author: piman <piman@chromium.org>
Date: Tue Mar 28 01:27:20 2017

Correctly destroy returned front buffer after resize

When returning a front buffer with ReturnFrontBuffer, if the front buffer was
resized in the mean time, we were considering the returned mailbox as lost, and
never freeing it, causing a large GPU memory leak.
This patch fixes it.

BUG= 692074 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2779873002
Cr-Commit-Position: refs/heads/master@{#459969}

[modify] https://crrev.com/15176a8d134862de21fe2de9f9fd1d7390ac9003/gpu/command_buffer/service/gles2_cmd_decoder.cc

Is there a test version of chrome which contains this fix?

Comment 17 by piman@chromium.org, Mar 30 2017

The current canary should have the fix. Anything after 59.0.3054.0.

Comment 18 by piman@chromium.org, Mar 30 2017

Labels: Merge-Request-58
Project Member

Comment 19 by sheriffbot@chromium.org, Mar 30 2017

Labels: -Merge-Request-58 Hotlist-Merge-Approved Merge-Approved-58
Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 20 by bugdroid1@chromium.org, Mar 30 2017

Labels: -merge-approved-58 merge-merged-3029
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/49aa6162abfe9749ead405c8f593abdea54ad630

commit 49aa6162abfe9749ead405c8f593abdea54ad630
Author: Antoine Labour <piman@chromium.org>
Date: Thu Mar 30 18:57:01 2017

Correctly destroy returned front buffer after resize

When returning a front buffer with ReturnFrontBuffer, if the front buffer was
resized in the mean time, we were considering the returned mailbox as lost, and
never freeing it, causing a large GPU memory leak.
This patch fixes it.

BUG= 692074 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2779873002
Cr-Commit-Position: refs/heads/master@{#459969}
(cherry picked from commit 15176a8d134862de21fe2de9f9fd1d7390ac9003)

Review-Url: https://codereview.chromium.org/2783203004 .
Cr-Commit-Position: refs/branch-heads/3029@{#495}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[modify] https://crrev.com/49aa6162abfe9749ead405c8f593abdea54ad630/gpu/command_buffer/service/gles2_cmd_decoder.cc

Comment 21 by piman@chromium.org, Mar 30 2017

Status: Fixed (was: Assigned)

Sign in to add a comment