1330173 - thread safety assertion for TabChildBase using DevTools

Status: RESOLVED INCOMPLETE

(Core :: DOM: Content Processes, defect, P3)

Description

[Jerri Rice (rehash pending)]

Attachment: firefoxMemoryLeak-nightly-01-10-17.png

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:50.0) Gecko/20100101 Firefox/50.0
Build ID: 20161208153507

Steps to reproduce:

I fired up firefox and opened the developer tools on a normal webpage.  During this the developer tools begin to fail to operate correction with console output listing:

[GPU 9032] WARNING: pipe error: 109: file c:/Users/[redacted]/mozilla-central/ipc/chr
omium/src/chrome/common/ipc_channel_win.cc, line 346

Once this happens, either triggering a crash or more simply pressing ctrl+c(for simplicity to see this) will result in something like the attached screenshot.  The screenshot is modified to remove my username etc, but I think demonstrates the problem fine.


Actual results:

Memory leaks, and even when trying to output information about the memory leak, ff is leaking more memory.

This issue can be hard to reproduce, so I'm trying to put together an automated testcase to show it off, but I have encountered it three times rapidly.  Plus it seems to disclose program internals and could possibly disclose information which should remain private.


Expected results:

No memory leaks should occur, especially those of this type which could be used to disclose confidential information from inside the firefox program.

Comment 1

[Jerri Rice (rehash pending)]

eww, operate correctly*

Thought I should mention this seems like it could be quite serious so I'll do my best to run this down if it's not taken care of before this reaches release.

Comment 2

[Jerri Rice (rehash pending)]

Once someone touches this, I can send you an unmodified screenshot showing the order of things in console output.  As I've said before, in my experience obscurity > security :-)

Just let me know.

Comment 3

[Andrew McCreight [:mccr8]]

We need more than this to be able to do anything, like a test case or something.

Comment 4

[Andrew McCreight [:mccr8]]

Jerri showed me a full screenshot, and we're hitting a thread safety assertion for TabChildBase, so I'll move this over to IPC for now.

Comment 5

[Andrew McCreight [:mccr8]]

We might be able to figure something out from a stack for that assertion.

Comment 6

[Daniel Veditz [:dveditz]]

Let's make sure some DevTools folks can see this bug, too. Isn't it more likely to be someone mis-using threads than something in IPC itself?

Comment 7

[Andrew McCreight [:mccr8]]

I guess TabChildBase is actually DOM: Content Processes, not IPC.

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

I think I would need to see more specific STR and / or test case to better understand what's happening here.

Does the assertion include a stack?

Comment 9

[Jerri Rice (rehash pending)]

I will be getting to this after doing what I can on bug 1330687.  This seems less worrisome so backburner for now.

Thanks for sticking with me guys.

Comment 10

[Jerri Rice (rehash pending)]

Still on my list, and it seems that the one of the two variations of the STR I came up with for bug 1330687 might actually relate to this.

I'll be getting here hopefully tomorrow evening EST.

Comment 11

[Daniel Veditz [:dveditz]]

Calling this sec-audit for now, but if you dig in and find more info ping us via mail to upgrade the severity.

Comment 12

[Jerri Rice (rehash pending)]

I'm currently away from my workstation so this has to be followed up in a strange way.  I have an idea which will allow me to get back to this, but it's slower than I like.

Stay tuned.

Comment 13

[Andrew McCreight [:mccr8]]

Can you just get us the stack for the thread safety assertion? That alone might point out what is going wrong here.

Comment 14

[Jerri Rice (rehash pending)]

Getting back on it now.  This involved a tab crash and may also be linked to the pb assertions from other bugs.  Here's the iffy part, I don't have a full debug build with me right now so bear with me.  It wasn't even hard to reproduce.

dev tools show an incredible slowdown, get the warning about the pipe error involving ipc, trigger crash.

The last time I thought this was happening I simply hit ctrl+c and experienced the same result without triggering a crash.

If I can't reproduce now I'll assume changes in nightly have fixed it and run down the list of changes between then and now.  Either way I should have you in the right direction.

Comment 15

[Jerri Rice (rehash pending)]

Ok, back to this.  The best I can do beyond what I'm doing now is make time tomorrow afternoon EST to set down at my workstation and build a full debug build to get back into this.

I'll be continuing work through the rest of this evening as long as I can to try to reproduce this, but without the warning about ipc being issued it's kind of hard to know when to try to trigger a crash.

I *should* know if I can work that time into my schedule over the next hour or two.  I'll update as necessary.

Thanks for putting up with me guys :-)

Comment 16

[Jerri Rice (rehash pending)]

The time mentioned above is scheduled for tomorrow, and since I have one more issue to try to reproduce which I don't even have a bug filed for yet, switching focus to that for now.  Should have answers here tomorrow evening EST.

Comment 17

[Jerri Rice (rehash pending)]

For those interested in the hold up here, see bug 133258 for the work I could do locally here.

Comment 18

[Jerri Rice (rehash pending)]

Attachment: Despite not being a full debug build, this screenshots shows the broken pipe ipc warnings with dev tools involved.

I may have been able to get to that stacktrace without the scheduled time for later today, but since it's already scheduled and will give far more useful output, I'm going to stick to my plan.

I'm all over the place right now with bug work, and I'm trying to work fast and efficient because AFAIK I have less than 24 hours to close this out, and I still have two issues to float over googles way which needs work yet.

The good news here is the testcase from bug 133258 should make getting that stacktrace easy.  The screenshot clearly shows that this testcase can produce the same problems with the dev tools open, and after that it's a simple crash, or ctrl+c if I have to resort to that to show the memory leak.

Stay tuned.

Comment 19

[Andrew McCreight [:mccr8]]

I think he means bug 1333258, not bug 133258.

Comment 20

[Jerri Rice (rehash pending)]

Good catch, thanks.  That's right, bug 1333258.  Getting my workstation setup now for the stacktrace needed here.  I literally have less than 18 hours, and still have some things I want to get in order for google.  Meh, not enough hours in the day.

Let me get to this.

Comment 21

[Jerri Rice (rehash pending)]

Attachment: firefoxMemoryLeak-nightly-01-24-17-unmodified.png

Here is an unmodified screenshot of the same memory leak.  I'll attach a follow up screenshot of where firefox has hit a breakpoint.

After that I'll fire up visual studio and get a stack trace from it since one isn't listed on the command line.

Comment 22

[Jerri Rice (rehash pending)]

Attachment: firefoxMemoryLeak-nightly-01-24-17-breakpoint.png

Here is a screenshot of the breakpoint that is triggered.  Now to fire up visual studio for that stack trace.

Comment 23

[Jerri Rice (rehash pending)]

Attachment: nightlyHasExitedBeforeBeingDebugged.png

Strange, that when I went to debug the crash, nightly had somehow already exited.  The fact that no stack trace is listed before the memory leak, and now this tells me this might be above my head.

I'll try a few more times, but after that I still have a few more things for bug 1333258 and something for google and the clock is ticking!   Apparently I work much better on a ticking clock ;-)

Comment 24

[Jerri Rice (rehash pending)]

Oh yeah, any ideas as to why this might be happening like this?

Comment 25

[Jerri Rice (rehash pending)]

Clearing the NI flag for me as this is as much as I'm going to be able to bring.  If someone else can pick it up from here, or if not that's fine.

I've triggered the issue many times, and it's not that hard to reproduce so with a little time I'm sure someone else could see it as well.

Either way, this one was a bit of a headache.  Don't dig too deep on it.

Cheers

Comment 26

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

(In reply to Jerri Rice (rehash pending) from comment #25)
> I've triggered the issue many times, and it's not that hard to reproduce so
> with a little time I'm sure someone else could see it as well.

Can you be more specific about the STR? Reading comment 0, it seems like you're just opening developer tools and you get a crash.

Comment 27

[Jerri Rice (rehash pending)]

At the time Bill, that was correct.  It was a nightly build at the time, and I was simply loading a testcase relating to something else, and then opening the global console specifically.

removing the NI for me, sorry that took so long.

Comment 28

[Andrew Sutherland [:asuth] (he/him)]

Per comment 27 we don't have an STR and it seems likely this was fixed either incidentally or a more specific bug was filed since I don't believe we see problems like this anymore.