Closed
Bug 1393058
Opened 7 years ago
Closed 7 years ago
Unwanted CC/GC logs on Android
Categories
(Firefox for Android Graveyard :: General, defect)
Firefox for Android Graveyard
General
Tracking
(firefox55 wontfix, firefox56 fixed, firefox57 fixed)
RESOLVED
FIXED
Firefox 57
People
(Reporter: mccr8, Assigned: erahm)
References
Details
Attachments
(1 file)
1.39 KB,
patch
|
mccr8
:
review+
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
Apparently a number of users are getting GC/CC logs dumped onto their phones, which are taking up a lot of space: https://twitter.com/gauthamrandom/status/900360592233385986 The link in the first comment is from 2016. I'm not really sure what could be causing this. My only theory is that maybe the signal mechanism that was added for B2G is somehow triggering on Android. We could see if that still exists. If it does, we should probably remove it, as I don't think people have been using GC/CC logs much on mobile. (I'm filing this in Android so maybe an Android person will see it...)
Assignee | ||
Comment 1•7 years ago
|
||
It's almost certainly coming from nsMemoryInfoDumper [1]. The likely culprit is the GC/CC signal handler [2] (we have a fifo version, but that's opt-in). So that means we're getting spammed with SIGRTMIN + 2, which is pretty odd. It's also possible an add-on is hitting the nsIMemoryInfoDumper interface [3] although a cursory search of the addons repo doesn't show anything. The signal thing can be kind of useful for getting a memory report out of sad process, perhaps we should just disable on android? [1] http://searchfox.org/mozilla-central/rev/48ea452803907f2575d81021e8678634e8067fc2/xpcom/base/nsMemoryInfoDumper.cpp#582-587 [2] http://searchfox.org/mozilla-central/rev/48ea452803907f2575d81021e8678634e8067fc2/xpcom/base/nsMemoryInfoDumper.cpp#183-192 [3] http://searchfox.org/mozilla-central/rev/48ea452803907f2575d81021e8678634e8067fc2/xpcom/base/nsIMemoryInfoDumper.idl#122-153
Reporter | ||
Comment 2•7 years ago
|
||
Oh, yeah, that is what I was thinking of. Thanks for tracking it down. Yeah, I'd just disable it on Android. It also sounds like this log is getting dumped somewhere that isn't cleared out regularly, which is not great. So even if you just got this occasionally, it would keep accumulating.
Comment 3•7 years ago
|
||
Please file an additional bug to delete any existing logs, and have it block Bug 1042369. Hooray for less storage impact!
Blocks: 1042372
Comment 4•7 years ago
|
||
We seem to not be checking whether there's an existing signal handler: http://searchfox.org/mozilla-central/rev/48ea452803907f2575d81021e8678634e8067fc2/xpcom/base/nsDumpUtils.cpp#155 This is a moot point if we're going to turn off the memory reporter signal stuff, but if this had been checking for an existing handler — and if this really is caused by signals — then we probably would have caught it long ago. (Realtime signals terminate the process by default, so if something was sending them then there must have been a handler.) See also bug 1038900, which happened when we hard-coded a signal number but *did* check for existing handlers, and in which some Android thing — possibly a vendor-specific proprietary driver; we never did find out exactly what — was using a dynamically allocated realtime signal and could race with our startup. Bug 1274873 is also somewhat related.
Assignee | ||
Comment 6•7 years ago
|
||
(In reply to Andrew McCreight [:mccr8] from comment #5) > Is this something you could fix, Eric? Yeah I'll just ifdef android out.
Assignee: nobody → erahm
Assignee | ||
Comment 7•7 years ago
|
||
It appears some Android devices like to send spurious RT signals to our process which we interpret to mean a gc/cc log is being requested. This causes large files to pile up in the device storage. We can just disable this feature for Android as it would be pretty hard for a user to actually use (they can just go to about:memory). Automation can still enable the FIFO queue if we ever want to start dumping memory reports on device again.
Attachment #8903820 -
Flags: review?(continuation)
Assignee | ||
Comment 8•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c6f6cb7207febec2690dc6311d83fd051b0e1b63
Reporter | ||
Comment 9•7 years ago
|
||
Comment on attachment 8903820 [details] [diff] [review] Disable nsMemoryInfo dumper RT signal handling on Android Review of attachment 8903820 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!
Attachment #8903820 -
Flags: review?(continuation) → review+
Assignee | ||
Comment 10•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/a2ff82d51552f3e8724b72e7801f88a993c536f6 Bug 1393058 - Disable nsMemoryInfo dumper RT signal handling on Android. r=mccr8
Comment 11•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a2ff82d51552
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(erahm)
Reporter | ||
Comment 13•7 years ago
|
||
Comment on attachment 8903820 [details] [diff] [review] Disable nsMemoryInfo dumper RT signal handling on Android Approval Request Comment [Feature/Bug causing the regression]: very old [User impact if declined]: Storage filling up on people's phones, under some unknown conditions. We've seen lots of complaints about this. See: https://forum.xda-developers.com/s7-edge/help/gc-cc-logs-mozilla-consuming-huge-space-t3397783 [Is this code covered by automated tests?]: no [Has the fix been verified in Nightly?]: No. We're not 100% sure it will fix it, but I don't know what else it could be. I've asked Dietrich to see if he still sees the issue with this patch. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: This disables a feature used only for debugging, and only on Android. [String changes made/needed]: none
Attachment #8903820 -
Flags: approval-mozilla-beta?
Updated•7 years ago
|
status-firefox55:
--- → wontfix
status-firefox56:
--- → affected
Comment 14•7 years ago
|
||
Comment on attachment 8903820 [details] [diff] [review] Disable nsMemoryInfo dumper RT signal handling on Android Let's uplift this for beta 10 to prevent an annoying problem for Fennec users.
Attachment #8903820 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 15•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/7f6aade38935
Comment 16•7 years ago
|
||
Hi, Is there any way this can be verified on mobile? I've done some research and found some information but for Firefox desktop
Reporter | ||
Comment 17•7 years ago
|
||
(In reply to Mihai Ninu {:Ninu} from comment #16) > Is there any way this can be verified on mobile? I've done some research and > found some information but for Firefox desktop Unfortunately, we don't know how to reproduce this. The closest we have is Dietrich over in bug 1397571 who was seeing this on his phone, but it wasn't clear how often it was actually triggered.
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•