Closed Bug 1241370 Opened 4 years ago Closed 4 years ago

Don't strdup() the message name in InterruptFrame

Categories

(Core :: IPC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: njn, Assigned: njn)

Details

Attachments

(1 file)

I did some cumulative heap profiling with DMD. While playing an 8 minute video
on YouTube on Mac, 1.7% (438,000) of all calls to malloc were due to the
strdup() call in InterruptFrame()'s constructor. Those strings are only used in
IPC error messages.

The strings don't need to be strdup()'d because Message::name_ is always a
string literal:

- The Message() constructor that sets name_ is always passed a string literal
  (mostly in IPDL-generated code), or nothing, in which case "???" is
  used.

- Message::set_name() is also always passed a string literal (again, mostly in
  IPDL_generated code).

It would be nice if we could guarantee that it's always a string literal and
thus has the same lifetime as the program, but I don't think that's possible in
C++.

Also, the lifetime of InterruptFrame objects appears to be the same as that of
CxxStackFrame objects (because they're created in CxxStackFrame's ctor and
destroyed in CxxStackFrame's dtor), and CxxStackFrame is a stack-only class,
and it's always instantiated in a function that takes a |Message| as an
argument, so even if some of the names were not string literals I think the
lifetimes work out safely.
Comment on attachment 8710246 [details] [diff] [review]
Don't strdup() the message name in InterruptFrame

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

Also, we only ever use the string if we're about to crash :-). Nice find.
Attachment #8710246 - Flags: review?(wmccloskey) → review+
https://hg.mozilla.org/mozilla-central/rev/5d12e137f2c1
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.