812648 - [meta] Fix memory leaks for WebRTC crashtests/mochitests

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P1)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

We have to fix all the memory leaks we can find in WebRTC code so that we can enable more testsuites and add real Mochitests.

This is a tracking bug so we know what has to be done. Without that fixed I'm not sure how many Mochitests we will be able to create and get checked-in.

For the existent crashtests I will try to get the leaks sorted out and analyzed as best as I can in the next days. I will file new bugs for each of them.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: BLOATLOG for WebRTC crashtests

If I run the crashtests via the following command I get this massive bloatlog:

TEST_PATH=dom/media/tests/crashtests/crashtests.list make -C ../obj/nightly/ crashtest

It's only happening for WebRTC but not for other crashtests! Running crashtests for offline mode I don't get any leakage at all. So not sure what's going on here. I will investigate it and spun new bugs for whatever I find.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I just have to add that I got the above bloatlog from an empty crashtests.list file! There was actually not a single crashtest run. So it's most likely related to the framework?

Comment 3

[Maire Reavy [:mreavy]]

Removed [blocking-gum+].  Bug 823520 tracks memory leaks blocking gUM.  We will likely unpref gUM before all of WebRTC.

Comment 4

[Jason Smith [:jsmith]]

So all dependencies are officially fixed. What's left to do here to be able to run these tests with leak checking? Or are we good here?

Comment 5

[Maire Reavy [:mreavy]]

This is not a blocker on its own; if any new leaks show up in mochitests, they'd be blockers

Comment 6

[Jason Smith [:jsmith]]

Looks like we still have leaks in the peer connection tests per a try run in bug 814718. Bug coming shortly...

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I pushed this to try with leak checking enabled for webrtc crashtests:
https://tbpl.mozilla.org/?tree=Try&rev=1bd6312b7f38

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Sorry, the last comment was intended for bug 814531.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

As long as bug 822956 isn't fixed we can't turn on mochitests. As first step I will work on the crashtests so we can get those enabled on m-c.

Comment 10

[Jason Smith [:jsmith]]

(In reply to Henrik Skupin (:whimboo) from comment #9)
> As long as bug 822956 isn't fixed we can't turn on mochitests. As first step
> I will work on the crashtests so we can get those enabled on m-c.

That bug in particular just involves the fact that patches mid way being built ended up having leaks. If they land what they have, then yes, we'll have leaks. But I think they are going to fix it before it lands.

Are there any remaining leaks for the existing state of the world for peer connection?

If not, I think we can close this.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I will wait for bug 822956 just for safety. If everything turns green I'm happy to close this bug and also enable the formerly leaking mochitests.

Comment 12

[Jason Smith [:jsmith]]

(In reply to Henrik Skupin (:whimboo) from comment #11)
> I will wait for bug 822956 just for safety. If everything turns green I'm
> happy to close this bug and also enable the formerly leaking mochitests.

Okay. Note that the mochitests for peer connection still won't be able to be enabled on central by default - we're still hitting crashes on central that probably need to be fixed first before we turn them on in CI by default.

Comment 13

[Jason Smith [:jsmith]]

We've landed mochitests on central and crashtests now on central. We've got bugs filed for followups on potential outstanding issues that exist, which are being tracked in other means. I don't see a reason to keep this bug open anymore - let's just use mlk in the keywords with a reference to the crashtest/mochitest for any remaining leaks.

Closing this out then.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Given that we fixed all of the referenced dependencies on this bug we also fixed the meta bug itself. For still open or newly filed issues we can use the mlk keyword for sure.