Closed Bug 688233 Opened 8 years ago Closed 8 years ago

about:memory blank/empty when /proc/self/smaps is missing

Categories

(Toolkit :: about:memory, defect)

9 Branch
x86
Linux
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla9

People

(Reporter: aceman, Assigned: justin.lebar+bug)

References

Details

(Keywords: regression, Whiteboard: [testday-20110923])

Attachments

(1 file)

Mozilla/5.0 (X11; Linux i686; rv:9.0a1) Gecko/20110921 Firefox/9.0a1 ID:20110921030906

about:memory page is blank for me (grey background with white rounded rectangle).
In safe mode too.
Mozilla/5.0 (X11; Linux i686; rv:9.0a1) Gecko/20110922 Firefox/9.0a1

WFM-Ubuntu 11.04
Mozilla/5.0 (X11; Linux i686; rv:9.0a1) Gecko/20110921 Firefox/9.0a1 

Works fine with ID:20110921030906 too.
I see this error in Error console at each reload of about:memory:

Error: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIMemoryMultiReporter.collectReports]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://global/content/aboutMemory.js :: getReportersByProcess :: line 288"  data: no]
Component: Developer Tools → about:memory
Product: Firefox → Toolkit
QA Contact: developer.tools → about.memory
Whiteboard: [MemShrink]
I get this in safe mode and also in completely new profile, also on nightly of 20110923.
I am using linux Slackware 13.37 distro, 32bit, kernel 3.0.3.
about:memory works fine on the same machine using FF6.0.2 and Aurora8 (updated daily).
It would be helpful if you could bisect this to figure out when it broke.  (It's pretty easy to bisect using mozregression [1].)

[1] https://github.com/harthur/mozregression
OK, found this result using the tool:

Last good nightly: 2011-09-08
First bad nightly: 2011-09-09

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b7d269a291b6&tochange=b57d50c6c046
Three about:memory commits in that changeset that I can see.

http://hg.mozilla.org/mozilla-central/rev/fd50c3e8aad3   Bug 684339 - about:memory should link to about:support; r=njn 
http://hg.mozilla.org/mozilla-central/rev/aa0168e8e389   Bug 684323 - In about:memory, set text-overflow: ellipsis when in non-verbose mode, and set max-width: 100% when in verbose mode. r=njn 
http://hg.mozilla.org/mozilla-central/rev/45b321b68c2f   Bug 674290 - Expose contents of /proc/self/maps and smaps in about:memory. r=njn
Yes, in that range, bug 674290 seems interesting.
I do not have any smaps files in /proc/<pid>/ . I may attach the maps file from a running Firefox process if needed.
Seems like a likely culprit.  I'll mark this as blocking that unless Justin determines otherwise.
Blocks: 674290
I have compiled the kernel myself, maybe I disabled some feature (or hidden some information for security reasons) and the file does not contain what you expect. I can provide kernel config, just tell what you need.
It could just be that the original patch didn't account for the possibility of this being hidden, or something.  I don't know anything about this code, and Justin is probably off work until Monday, but I'm sure he'll get back to you with questions then.  Thanks again for the bug report.
Whiteboard: [MemShrink] → [MemShrink][testday-20110923]
Summary: about:memory blank/empty in Firefox/9.0a1 ID:20110921030906 → about:memory blank/empty when /proc/self/smaps is missing
Assignee: nobody → justin.lebar+bug
Whiteboard: [MemShrink][testday-20110923] → [testday-20110923]
Attached patch Patch v1Splinter Review
Attachment #562484 - Flags: review?(khuey)
I considered making the MapsMemoryReporter not throw an error when it can't find /proc/self/smaps, but that doesn't make much sense.  It *is* a failure.  But about:memory should be able to handle the failure instead of just giving up.
Kyle, are you able to look at this today?  I'd like to get this in before we branch, since the smaps parsing is new to FF9.
(In reply to Justin Lebar [:jlebar] from comment #15)
> Kyle, are you able to look at this today?  I'd like to get this in before we
> branch, since the smaps parsing is new to FF9.

Looking at it now.
Comment on attachment 562484 [details] [diff] [review]
Patch v1

Review of attachment 562484 [details] [diff] [review]:
-----------------------------------------------------------------

Heh, I expected something more complicated.

What does this actually look like when things fail?
Attachment #562484 - Flags: review?(khuey) → review+
> What does this actually look like when things fail?

Ugly.  It puts 
 
  Error: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIMemoryMultiReporter.collectReports]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://global/content/aboutMemory.js :: getReportersByProcess :: line 288"  data: no]

at the top of the page.

I'm not sure what else to do, though.  The JS doesn't know that it's querying the smaps multi-reporter, since multi-reporters don't have any kind of name to speak of (maybe that's a bug), so we can't do something special for smaps.
(In reply to Justin Lebar [:jlebar] from comment #18)
> > What does this actually look like when things fail?
> 
> Ugly.  It puts 
>  
>   Error: uncaught exception: [Exception... "Component returned failure code:
> 0x80004005 (NS_ERROR_FAILURE) [nsIMemoryMultiReporter.collectReports]" 
> nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame ::
> chrome://global/content/aboutMemory.js :: getReportersByProcess :: line 288"
> data: no]
> 
> at the top of the page.

Yeah, that's pretty ugly.

> I'm not sure what else to do, though.  The JS doesn't know that it's
> querying the smaps multi-reporter, since multi-reporters don't have any kind
> of name to speak of (maybe that's a bug), so we can't do something special
> for smaps.

Why do we need to do something special here?  Why can't we just handle any multi-reporter failing?
It seems the smaps file is enabled with CONFIG_PROC_PAGE_MONITOR. It is possible this is enabled on standard distros. But I have it disabled on all kernels configured by myself as I haven't seen any use for it (no other program needs it).

I will enable it, if it is useful to Firefox. Would it be possible to write a hint into about:memory to propose to enable this option if Firefox detects the file is missing? I think testers using about:memory would likely enable it if they find it could be utilized.
Status: UNCONFIRMED → NEW
Ever confirmed: true
> Why do we need to do something special here?  Why can't we just handle any multi-reporter failing?

That's what the patch does.

But if we wanted to do something pretty, like say "You don't have /proc/self/smaps" or even completely ignore the error as benign, we'd have to know that it's specifically the smaps reporter that's failing.
(In reply to Justin Lebar [:jlebar] from comment #21)
> > Why do we need to do something special here?  Why can't we just handle any multi-reporter failing?
> 
> That's what the patch does.
> 
> But if we wanted to do something pretty, like say "You don't have
> /proc/self/smaps" or even completely ignore the error as benign, we'd have
> to know that it's specifically the smaps reporter that's failing.

I suppose ideally we'd have:

<foo measurement>: an error occurred during collection of this metric

or something.

Anyways, this is good enough to ship in 9 (IMO), especially given that this only affects a non-standard kernel configuration.
> I suppose ideally we'd have: <foo measurement>: an error occurred during collection of this metric, 
> or something.

Yeah, but to get that, we need to add a name to the multi-reporters.  Right now, if CollectReports fails, you might not get anything out of the multi-reporter.  It could be any reporter; you can't tell...

In any case, inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/fa1a7fb55799

aceman, thanks for the bug!  Kyle and I have been discussing (in a somewhat obfuscated way) the fact that it's hard, for silly architectural reasons, to print a nice error message saying "btw, you don't have /proc/self/smaps."  But you'll get an unhelpful error message with this patch!  :)

At least about:memory won't be blank.
Whiteboard: [testday-20110923] → [testday-20110923][inbound]
(In reply to Justin Lebar [:jlebar] from comment #23)
> > I suppose ideally we'd have: <foo measurement>: an error occurred during collection of this metric, 
> > or something.
> 
> Yeah, but to get that, we need to add a name to the multi-reporters.  Right
> now, if CollectReports fails, you might not get anything out of the
> multi-reporter.  It could be any reporter; you can't tell...

Right, which is why I think this is fine to land as is.  I think we can do better in the future though.  Just not with more work than I think is worth investing in this one edge case.
Well, if this new about:memory gets to more people, maybe you can find out how common this edge configuration is (I don't know, maybe just seftconfigured kernels).

If you could put that 'unhelpful message' and explanation on a wiki page on MDN, maybe it would be enough for people to find it there (via google).

Thanks for working on it, I can test the next nightly with the fix.
https://hg.mozilla.org/mozilla-central/rev/fa1a7fb55799
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [testday-20110923][inbound] → [testday-20110923]
Target Milestone: --- → mozilla9
IME, not having /proc/self/maps is unusual.

As for multi-reporters needing names:  yes.  It's something I've thought about but have waited until it was necessary.  This might be the motivating case -- care to file a follow-up bug, jlebar?
Was that a typo? :) We are talking about 'smaps', which I understand is an extended version of 'maps' (which I do have).
Filed bug 689583 on names for multi-reporters.
With todays nightly the about:memory is still empty but I get the error mentioned in comment 18 in Error console (not top of the page). Is that the intended behavior?
(In reply to aceman from comment #30)
> With todays nightly the about:memory is still empty but I get the error
> mentioned in comment 18 in Error console (not top of the page). Is that the
> intended behavior?

Yes.  Displaying a different error (or displaying none at all) is blocked on bug 689583.
So what is actually the gain from this patch? Only the message as an indicator of problems of some unknown sort?

I thought this smaps reporter would throw the error but the other ones would be displayed normally.
Ohgosh, I'm sorry!  I completely misread your comment.

That's *not* the expected behavior.  But I don't think today's nightly has this patch.  The 9/28 nightly should.
OK, in today's Nightly 10a1 it works better, the error is on the top of about:memory (not in Error console) and the rest of the reporters is working fine. The user does not really know what he is missing. But at least the usual entries are there.
Verifying, thanks for the fix.
Status: RESOLVED → VERIFIED
I also confirm that enabling the kernel config option makes the error in Firefox go away and the about:memory page shows sections like 'Resident Set Size (RSS) Breakdown'.
Blocks: 715025
You need to log in before you can comment on or make changes to this bug.