large cores left behind by ui.ChromeCrashLoggedIn and ui.ChromeCrashNotLoggedIn |
||||
Issue descriptionFrom 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?)
,
Sep 7
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.
,
Sep 7
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.
,
Sep 7
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!
,
Sep 7
You ought to be able to see the behavior pretty easily with a VM, no?
,
Sep 7
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.
,
Sep 7
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.
,
Sep 7
#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.)
,
Sep 7
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.
,
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
,
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
,
Sep 11
Please let me know if you still see any files sticking around.
,
Sep 11
Will do, thnx!
,
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
,
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
,
Sep 13
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 |
||||
Comment 1 by achuith@chromium.org
, Sep 7