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

Issue 766131 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 765656



Sign in to add a comment

AsyncMethodRunner cannot be GC'd and keeps users of it alive

Project Member Reported by hbos@chromium.org, Sep 18 2017

Issue description

AsyncMethodRunner contains a Timer and the Timer has a RefPtr back to the AsyncMethodRunner. This is a cyclic reference that keeps AsyncMethodRunner from being GC'd.

Because AsyncMethodRunner cannot be GC'd, classes that use it cannot be GC'd either because the AsyncMethodRunner has a Member to the class whose method it should be invoked.

For example, MediaRecorder cannot be GC'd:

  getUserMedia({ audio: true, video: true },
      function(stream) {
        let recorder = new MediaRecorder(stream, {});
        recorder = null;
        gc();  // This should GC the |MediaRecorder| but it doesn't
      },
      function(error) {
        throw failTest('getUserMedia failed: ' + error);
      });

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/mediarecorder/MediaRecorder.h?q=MediaRecorder.h&sq=package:chromium&dr=CSs&l=114

Or anything with a "dispatch_scheduled_event_runner_". RTCPeerConnection is another example which can consume large amounts of resources, https://crbug.com/765656.
 

Comment 1 by hbos@chromium.org, Sep 18 2017

Description: Show this description

Comment 2 by hbos@chromium.org, Sep 18 2017

Cc: hbos@chromium.org
Please take a look or re-triage tyoshino.

Comment 3 by hbos@chromium.org, Sep 27 2017

Friendly ping.

Comment 4 by hbos@chromium.org, Oct 3 2017

Cc: tyoshino@chromium.org
Owner: ----
Status: Untriaged (was: Assigned)
Unassigning so that it can be triaged by Blink>Internals folks.

Comment 5 by hbos@chromium.org, Oct 10 2017

Components: -Blink>Internals Blink

Comment 6 by junov@chromium.org, Oct 11 2017

Cc: junov@chromium.org
Components: -Blink Blink>MediaRecording
The Timer's reference back to AsyncMethodRunner is a raw pointer: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/Timer.h?sq=package:chromium&dr=CSs&l=167

Even if it were a Member<>, that would not be a problem.  Unreachable reference cycles get GC'ed. Only a Persistent<> would cause a reference cycle that keeps the object alive forever. So I don't see a problem here. Am I missing something?

In the above example involving MediaRecorder, I suspect there might be something else keeping the object alive. I took a quick peek at MediaStream: AFAICT, it correctly uses WeakMember<> to hold references to registered observers (the media recorders?), so that's not the problem...

Triaging to MediaRecording component for further investigation.

Comment 7 by hbos@chromium.org, Oct 11 2017

You're right about cyclic Members being fine and this one being a raw pointer and using WeakPtrFactory in other places, which is also fine. It's not a Persistent or a RefPtr as claimed in the bug description. So I'm not sure what I was referring to... maybe I made a mistake.

I can't see what the problem is from codesearch, but I was able to make RTCPeerConnection garbage collectable by removing the use of dispatching events with a Member<AsyncMethodRunner<RTCPeerConnection>>.
Sorry for not responding. I basically agree with junov@.

How are you checking that a certain object is GC-ed? Printing a log? DevTools?
 Doesn't adding setTimeout to give a little time for Chrome make GC happen?

Comment 9 by hbos@chromium.org, Oct 13 2017

printf("OBJECT GARBAGE COLLECTED\n") in the destructor. I've not had to setTimeout before (isn't GC instant?), I've been running browser_tests which jump between processes before terminating. I don't have time now but if you need me to I could try to repro?
Cc: emir...@chromium.org chfremer@chromium.org
Status: Available (was: Untriaged)
Cc: -junov@chromium.org

Sign in to add a comment