New issue
Advanced search Search tips

Issue 881638 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 11
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocking:
issue 876587



Sign in to add a comment

large cores left behind by ui.ChromeCrashLoggedIn and ui.ChromeCrashNotLoggedIn

Project Member Reported by achuith@chromium.org, Sep 7

Issue description

From bpastene:
There seems to be a sizable leak in the new tast tests we've started running. It looks like two of them (ui.ChromeCrashLoggedIn and ui.ChromeCrashNotLoggedIn) intentionally cause the browser to crash, which creates crash dumps at both "/var/spoole/crash/" and "/home/chronos/crash/". Looking at the logs, the test runner cleans up the ones at /home/chronos/crash/ but not the ones at /var/spoole/crash/, which leaves ~400MB files that look like "chrome.20180906.155826.15036.core" lying around. Could we get those cleaned up as well? (Or alternatively, not generated in the first place?)
 
Blocking: 876587
Cc: vapier@chromium.org
Components: OS>Systems Tests>Tast
Labels: OS-Chrome
Hmm, I don't see this when I run the tests on a recent caroline build. Are core dumps perhaps enabled on the VM images that are being used here? Does /mnt/stateful_partition/etc/enable_chromium_coredumps exist, for instance? (See issue 881172, which Mike just filed.)

If that file exists in the VMs, I could make these tests temporarily remove it as a stopgap measure.
i don't think this is enable_chromium_coredumps ... if it was, you'd see large dumps in /var/coredumps/ rather than /var/spool/crash/.  the latter is only managed by crash-reporter.

all dev/test images will save .core files (due to /root/.leave_core).  but Chrome crashes normally only get processed when /mnt/stateful_partition/etc/collect_chrome_crashes exists, and only autotests create that file.  so i'm guessing you ran an autotest at some point.

i'm not seeing any logic in autotest that clears that flag.  we should fix that.

but if tast is clearing crashes, it probably should be clearing it from all spool dirs.
Status: Started (was: Assigned)
I still haven't managed to configure my caroline device (with a dev image) to write Chrome core files to /var/spool/crash, but I've uploaded https://crrev.com/c/1212543 as a speculative fix. Please let me know if it fixes this (or doesn't). Thanks!
You ought to be able to see the behavior pretty easily with a VM, no?
i don't think a fresh VM would run into this, or one where Chrome doesn't crash.  but if you ran some crash-related autotest, and then started triggering Chrome crashes, you'd get into this state.

the autotest framework has some hooks to purge all crash spool dirs before it runs, and collect all crashes after the test finishes (in case something random went wrong), so having tast do the same prob makes sense.
Looks like I indeed needed to run some Autotest-based tests (via vm_sanity.py) to put the VM into this state. :-/

The change I sent appears to fix the issue with core files being left around. There are still smaller .log and .meta files left behind in /var/spool/crash, but I don't know if those are a problem or not. If they are, a better fix might be to make the test undo whatever Autotest is doing.
#6: Tast currently collects new crashes from DUTs, although it doesn't delete them from the DUT afterward. Tast is currently wrapped within an Autotest-based test in order to run in the lab, and I didn't want to step on Autotest's toes.

(This wouldn't help for vm_sanity.py in any case, since it's running local_test_runner directly instead of running the tast executable, which is the thing that handles collecting logs, crashes, etc.)
i've moved the autotest cleanup to issue 882004.  so once tast has mitigations in place (which imo it should anyways if it's going to crawl any crash spool dir), you can close this out.
Project Member

Comment 10 by bugdroid1@chromium.org, Sep 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/tast-tests/+/cff6594ff5344d347bdb48aa7ef81fda92baff67

commit cff6594ff5344d347bdb48aa7ef81fda92baff67
Author: Daniel Erat <derat@chromium.org>
Date: Tue Sep 11 17:02:06 2018

tast-tests: Make ui.ChromeCrash* delete all new crashes.

Update the ui.ChromeCrashLoggedIn and
ui.ChromeCrashNotLoggedIn tests to delete all new
crash-related files written to /home/chronos/crash and
/var/spool/crash.

These tests attempt to only produce minidump files in
/home/chronos/crash, but apparently some Autotest-based
tests run by vm_sanity.py configure DUTs to also write
Chrome core files to /var/spool/crash, and we don't want to
leave a bunch of bulky files lying around if one of those
tests runs before the Tast tests.

BUG= chromium:881638 
TEST=tests still pass; ran ui.ChromeCrash* against a VM
     where i'd previously run vm_sanity.py and verified that
     /var/spool/crash and /home/chronos/crash are empty
CQ-DEPEND=I45c331036842993f93fd9b982abf6a449d7a15c4

Change-Id: I3d94232690c296710608cf5020bc49f41017ff74
Reviewed-on: https://chromium-review.googlesource.com/1212543
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Shuhei Takahashi <nya@chromium.org>

[modify] https://crrev.com/cff6594ff5344d347bdb48aa7ef81fda92baff67/src/chromiumos/tast/local/bundles/cros/ui/chromecrash/crash.go

Project Member

Comment 11 by bugdroid1@chromium.org, Sep 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/tast/+/95c1c83b320446f331367f8038c68caea0e153ff

commit 95c1c83b320446f331367f8038c68caea0e153ff
Author: Daniel Erat <derat@chromium.org>
Date: Tue Sep 11 17:02:06 2018

tast: Make crash.GetCrashes return all crash-related files.

Update crash.GetCrashes to return a single slice of all
crash-related files (.core, .dmp, .log, .meta) and move
filtering to callers.

BUG= chromium:881638 
TEST=unit tests pass; also updated a test to send SEGV to
     chrome and checked that the crash is still collected by
     "tast run"
CQ-DEPEND=I3d94232690c296710608cf5020bc49f41017ff74

Change-Id: I45c331036842993f93fd9b982abf6a449d7a15c4
Reviewed-on: https://chromium-review.googlesource.com/1214783
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Shuhei Takahashi <nya@chromium.org>

[modify] https://crrev.com/95c1c83b320446f331367f8038c68caea0e153ff/src/chromiumos/tast/crash/crash_test.go
[modify] https://crrev.com/95c1c83b320446f331367f8038c68caea0e153ff/src/chromiumos/cmd/local_test_runner/main.go
[modify] https://crrev.com/95c1c83b320446f331367f8038c68caea0e153ff/src/chromiumos/tast/crash/crash.go
[modify] https://crrev.com/95c1c83b320446f331367f8038c68caea0e153ff/src/chromiumos/tast/runner/sys_info.go

Status: Fixed (was: Started)
Please let me know if you still see any files sticking around.
Will do, thnx!
Project Member

Comment 14 by bugdroid1@chromium.org, Sep 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/tast-tests/+/dc9782f0631b955dfb154d6a48c7023bf5bb4d17

commit dc9782f0631b955dfb154d6a48c7023bf5bb4d17
Author: Daniel Erat <derat@chromium.org>
Date: Thu Sep 13 08:23:10 2018

tast-tests: Make chrome package use crash.ChromeCrashDir.

Make the chrome package reference the crash package's
ChromeCrashDir constant when setting BREAKPAD_DUMP_LOCATION
instead of duplicating the /home/chronos/crash path.

BUG= chromium:881638 
TEST=ui.ChromeCrash* tests still pass

Change-Id: I9e1f7f3ccb79b61cc2cd4121fbf30657e77ec2c0
Reviewed-on: https://chromium-review.googlesource.com/1220297
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Shuhei Takahashi <nya@chromium.org>

[modify] https://crrev.com/dc9782f0631b955dfb154d6a48c7023bf5bb4d17/src/chromiumos/tast/local/chrome/chrome.go

Project Member

Comment 15 by bugdroid1@chromium.org, Sep 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/tast/+/d4078875ea790a0b6286a47cee9c7ccd7592a9ab

commit d4078875ea790a0b6286a47cee9c7ccd7592a9ab
Author: Daniel Erat <derat@chromium.org>
Date: Thu Sep 13 08:23:05 2018

tast: Explain how Chrome minidump location is configured.

Add a comment to the crash package documenting how the
chrome package sets BREAKPAD_DUMP_LOCATION to instruct
Chrome to write minidumps to /home/chronos/crash.

BUG= chromium:881638 
TEST=none

Change-Id: I42748c1b56e69513314fe83a9c9797d6e40228ce
Reviewed-on: https://chromium-review.googlesource.com/1220296
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Shuhei Takahashi <nya@chromium.org>

[modify] https://crrev.com/d4078875ea790a0b6286a47cee9c7ccd7592a9ab/src/chromiumos/tast/crash/crash.go

Unfortunately the fixes here haven't made their way into the VM images chromium uses yet. So the disks are still getting filled with crash dumps. Uploaded https://chromium-review.googlesource.com/c/chromium/src/+/1225934 to disable the test until the fix rolls in.

Sign in to add a comment