The default bug view has changed. See this FAQ.

If using multiple processes, display pid along with NS_WARNINGs

RESOLVED FIXED in mozilla14

Status

()

Core
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Justin Lebar (not reading bugmail), Assigned: Justin Lebar (not reading bugmail))

Tracking

unspecified
mozilla14
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
Debugging multiprocess Firefox is complicated by the fact that NS_WARNINGs are not tagged with a pid.

We could unconditionally display the pid, but then that's added noise for the majority of people, who aren't developing for e10s.

As a compromise, I propose displaying the pid only when we've spawned a subprocess.

Does the plugin-container process output NS_WARNINGs?
They could, but since they don't use XPCOM I don't think they actually do.
What you propose sounds hard.  jduell made us process-specific NSPR log files, which should satisfy your use case here.
(Assignee)

Comment 3

5 years ago
> jduell made us process-specific NSPR log files, which should satisfy your use case here.

NS_Warning doesn't appear to be written via PR_Log, so this doesn't help much, unless I misunderstand.
(Assignee)

Comment 4

5 years ago
Created attachment 594797 [details] [diff] [review]
Patch v1
(Assignee)

Updated

5 years ago
Attachment #594797 - Flags: feedback?(jones.chris.g)
(In reply to Justin Lebar [:jlebar] from comment #3)
> > jduell made us process-specific NSPR log files, which should satisfy your use case here.
> 
> NS_Warning doesn't appear to be written via PR_Log, so this doesn't help
> much, unless I misunderstand.

Nope, you don't.  My understanding was wrong.
Comment on attachment 594797 [details] [diff] [review]
Patch v1

Looks good.  Use GetCurrentProcId() from base/process_util.h for the tagging.
Attachment #594797 - Flags: feedback?(jones.chris.g) → feedback+
(Assignee)

Updated

5 years ago
Depends on: 725519
(Assignee)

Comment 7

5 years ago
After working with this for a few days, I want to display "Parent" or "Child" along with the PID.  But doing that requires calling ClearOnShutdown from this function I'd like to be thread-safe (so we don't leak the description string).  Which requires making ClearOnShutdown thread-safe.

Thus, bug 725519.
Why is that necessary, can't those be static strings?
(Assignee)

Comment 9

5 years ago
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #8)
> Why is that necessary, can't those be static strings?

We could require that the char* passed for the string be statically-allocated.  I can't enforce that with the type system, so I figured I wouldn't add a footgun opportunity to the API.  But maybe it's worth the hack.

We should probably make ClearOnShutdown thread-safe anyway.
(Assignee)

Comment 10

5 years ago
Let's just make nsDebugImpl::SetMultiprocessMode main-thread only.  That'll simply things.
(Assignee)

Comment 11

5 years ago
Created attachment 613366 [details] [diff] [review]
Patch v2
Attachment #613366 - Flags: review?(benjamin)
(Assignee)

Updated

5 years ago
Attachment #594797 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Assignee: nobody → justin.lebar+bug
(Assignee)

Comment 12

5 years ago
Created attachment 613369 [details] [diff] [review]
Patch v2.1

Remove unnecessary include.
Attachment #613366 - Attachment is obsolete: true
Attachment #613366 - Flags: review?(benjamin)
Attachment #613369 - Flags: review?(benjamin)
Comment on attachment 613369 [details] [diff] [review]
Patch v2.1

So this will not have any effects for plugin parent/child relations, only content? That's ok, although it seems a bit surprising that simply creating a ContentParent will change the output of NS_WARNING in general.
Attachment #613369 - Flags: review?(benjamin) → review+
https://hg.mozilla.org/mozilla-central/rev/91bbeb1c5187
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
You need to log in before you can comment on or make changes to this bug.