Crash properly after record/replay initialization failures

RESOLVED FIXED in Firefox 64

Status

()

defect
RESOLVED FIXED
8 months ago
7 months ago

People

(Reporter: bhackett, Assigned: bhackett)

Tracking

Trunk
mozilla64
Points:
---

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

8 months ago
Posted patch patchSplinter Review
If record/replay initialization fails (usually due to not being able to set up redirections) then we end up crashing shortly afterwards, but this happens before connecting to the middleman, which ends up hanging and not reporting anything useful.  The attached patch fixes things so that the child process lives long enough to report the error it encountered to the middleman.  The error is also printed to stderr immediately in case the fixes here regress again.
Attachment #9015098 - Flags: review?(nfroyd)
Comment on attachment 9015098 [details] [diff] [review]
patch

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

r=me with some kind of change like the below done.  Randomly checking gInitializationFailureMessage for what we're trying to do here is obscure at best.

::: toolkit/recordreplay/Lock.cpp
@@ +201,2 @@
>  
> +  if (!gInitializationFailureMessage) {

WDYT about commenting all changes in this patch similar to what was already done in Thread.cpp so that the intent behind these checks is more obvious?  (I considered suggesting some sort of function wrapper named to clarify the intent, but I couldn't come up with a good name...)
Attachment #9015098 - Flags: review?(nfroyd) → review+

Comment 2

7 months ago
Pushed by bhackett@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e36764125483
Crash properly after record/replay initialization failures, r=froydnj.

Comment 3

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e36764125483
Status: NEW → RESOLVED
Last Resolved: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.