New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 688248 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Feb 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

System crash reporter fails to process browser crash, doesn't get report id

Project Member Reported by jamescook@chromium.org, Feb 3 2017

Issue description

Google Chrome	56.0.2924.87 (Official Build) beta (64-bit)
Platform	9000.82.0 (Official Build) beta-channel samus

(Also happens with ToT chrome with a branded build on device.)

* Go to about:inducebrowsercrashforrealz to crash the browser process
* After browser restart, look in about:system under ui_log

You'll see some normal log lines:
[1349:1349:0202/213731.705352:ERROR:message_service.cc(469)] Failed to create native process.

Then at the time of the crash these three lines, without the usual prefix:
_sys_cr_finished
System crash-reporter failed to process crash report.
Report Id: 

Then a little later the browser comes back up and does some recovery:
[0202/213753.254492:WARNING:diagnostics_writer.cc(209)] [FAIL] 009 PathUserData (Path contents too large (8188902614) for: /home/chronos)

If you go to about:crashes there is no report (yet) for this browser crash.

The text _sys_cr_finished is kSuccessMagic in the system crash_reporter:
https://cs.corp.google.com/chromeos_public/src/platform2/crash-reporter/chrome_collector.cc?type=cs&q=_sys_cr_finished&sq=package:%5Echromeos_(public%7Cinternal)$&l=332

It is written to output_file_ptr_ which is initialized to stdout.

https://cs.corp.google.com/chromeos_public/src/platform2/crash-reporter/chrome_collector.cc?type=cs&q=_sys_cr_finished&sq=package:%5Echromeos_(public%7Cinternal)$&l=118

It is read in the chrome breakpad code here:

https://cs.chromium.org/chromium/src/components/crash/content/app/breakpad_linux.cc?type=cs&q=_sys_cr_finished&sq=package:chromium&l=1358

The value is read from here:

https://cs.chromium.org/chromium/src/components/crash/content/app/breakpad_linux.cc?type=cs&sq=package:chromium&l=1821

I don't understand what this code is doing with file descriptors. It looks like it is trying to hook up crash_reporter's output to a pipe. If that's true, however, then I would *not* expect to see the magic string printed to the logs.

I see this magic string was added in https://codereview.chromium.org/248653002 

I'm not sure if this causes issues in practice. Does chrome care if crash_reporter succeeded? If it doesn't care it shouldn't be printing errors to the log.

vapier / thestig - any ideas?

 
Summary: System crash reporter fails to process browser crash, doesn't get report id (was: System crash reporter fails to process browser crash, doesn't get report id, maybe delays upload?)
I don't think this delays upload - crash_sender just does that on a schedule. However, I think the breakpad code is doing something wrong with file descriptors or how it reads the success magic.

Advice welcome.

since chrome relies on crash_reporter to handle its crashes (i.e. queue+upload), it does care if it succeeds.  otherwise, crashes from chrome would get silently dropped on the floor.

i'm not super familiar with the breakpad integration/architecture on the chrome side.

afaik, the current CrOS behavior with chrome://crashes is to only list uploaded crashes.  we want to extend that to list queued-but-not-uploaded, but haven't found time to work on that yet.
I found the problem.

On Linux we run wget. We force it to write status to /dev/fd/3. We use dup2() to hook up fd 3 to a pipe that is read by a helper process. The helper processes uses that to get the crash id.

On Chrome OS we run crash_reporter. crash_reporter writes status to stdout, not to fd 3. So the output goes to the logs and the helper process prints an error.

I think this was broken when thestig@ added the _sys_cr_finished magic.

I'll have a CL to fix this up shortly.

Project Member

Comment 4 by bugdroid1@chromium.org, Feb 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bdead22a98f5f12e9ebc0d9356b5521f5e0be793

commit bdead22a98f5f12e9ebc0d9356b5521f5e0be793
Author: jamescook <jamescook@chromium.org>
Date: Tue Feb 07 20:41:26 2017

breakpad: Fix crash_reporter upload success handling on Chrome OS

Chrome OS incorrectly prints this message to the ui log after each crash:
"System crash-reporter failed to process crash report."
This makes it confusing to work on crash reporting, as crash_reporter is
actually succeeding.

The problem was how breakpad listens for the crash id / success string.
A helper process reads the crash id/status string from a pipe. That pipe
was connected to fd 3 via sys_dup2(). This worked on Linux because wget was
configured to write status to /dev/fd/3. This failed on Chrome OS because
crash_reporter writes status to stdout (fd 1).

This CL wires crash_reporter's output to stdout using sys_dup2(). It also
eliminates the dup2() code on Linux, because wget can write directly to
the pipe to the helper process.

I also pulled out some helper functions because the existing functions are
too long, and also added some docs and error checking.

BUG= 688248 
TEST=Navigate to chrome:inducebrowsercrashforrealz on chromebook, look at
/var/log/ui/ui.LATEST, see "Crash dump received by crash_reporter" log line.

Review-Url: https://codereview.chromium.org/2676023003
Cr-Commit-Position: refs/heads/master@{#448724}

[modify] https://crrev.com/bdead22a98f5f12e9ebc0d9356b5521f5e0be793/components/crash/content/app/breakpad_linux.cc

Status: Fixed (was: Assigned)
Cc: abodenha@chromium.org ronaldho@chromium.org textor@chromium.org moch@chromium.org stevenh@chromium.org harpreet@chromium.org
 Issue 597848  has been merged into this issue.
Cc: ka...@chromium.org

Comment 8 by son...@google.com, Mar 3 2017

Status: Verified (was: Fixed)
Verified on build 9334.0.0
Verified on Samus with version:  58.0.3028.0/9334.0.0 (Official Build) dev-channel

Sign in to add a comment