Chromedriver blindly clobbers all directories in "/tmp". |
||||||
Issue descriptionFrom I'm working on testing a new logging system on Chromium waterfalls. This includes bootstrapping builds through a logging client proxy, which owns state in "/tmp/.recipe_engine". The Chromedriver script ( https://chromium.googlesource.com/chromium/src/+/master/chrome/test/chromedriver/run_buildbot_steps.py#388 ) has a section that forcefully deletes *every* subdirectory under "/tmp", including "/tmp/.recipe_engine", leading to some nasty breakages. I'm not sure if blindly clobbering everything under "/tmp" has *ever* been safe in a Linux system, but it definitely wrecks the ownership expectations of "annotated_run.py". I'd like to remove the clobbering from the script. Can someone who is more familiar with the script tell me if this is a good idea? +people who have worked on the script.
,
Mar 28 2016
Looks like this was introduced in https://chromiumcodereview.appspot.com/14301024 +stgao, looks like this is doing a kill -9 on all the stray chrome processes, and attempting to delete any temporary profile directories. Is this correct?
,
Mar 28 2016
IIRC, yes. The cleanup of /tmp is because there were a lot of temporary files (created by Chromedriver for --user-data-dir that are used by chrome) that made the disk full, and caused breakage afterwards.
,
Mar 29 2016
I agree with dnj@ that going for all of /tmp/* is pretty questionable. Looking at the CL it seems it's specifically deleting *directories* in /tmp, but blindly, which is still not great. I think the right thing to do is: 1) modify src/chrome/test/chromedriver/util.py MakeTempDir() to pass the prefix parameter to mkdtemp, perhaps "chromedriver" 2) only delete directories starting with "chromedriver" in run_buildbot_steps.py both are 1 line edits. Thanks in advance to whoever writes the CL :)
,
Mar 29 2016
I'll have a go at making this change. But if these temp directories are profile directories, then these are created by the ChromeDriver binary itself, not in util.py - MakeTempDir() already calls atexit.register(MaybeDelete, path). ChromeDriver uses ScopedTempDir, which I think respects $TMPDIR, so maybe we can just set that before launching ChromeDriver.
,
Mar 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a05d65bcb2ac1339a99beaa22cf2e9ede5ff7d72 commit a05d65bcb2ac1339a99beaa22cf2e9ede5ff7d72 Author: samuong <samuong@chromium.org> Date: Wed Mar 30 18:37:53 2016 [chromedriver] Only delete temp files that start with "chromedriver_". BUG= 598366 Review URL: https://codereview.chromium.org/1844743002 Cr-Commit-Position: refs/heads/master@{#384032} [modify] https://crrev.com/a05d65bcb2ac1339a99beaa22cf2e9ede5ff7d72/chrome/test/chromedriver/run_buildbot_steps.py [modify] https://crrev.com/a05d65bcb2ac1339a99beaa22cf2e9ede5ff7d72/chrome/test/chromedriver/util.py
,
Mar 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a05d65bcb2ac1339a99beaa22cf2e9ede5ff7d72 commit a05d65bcb2ac1339a99beaa22cf2e9ede5ff7d72 Author: samuong <samuong@chromium.org> Date: Wed Mar 30 18:37:53 2016 [chromedriver] Only delete temp files that start with "chromedriver_". BUG= 598366 Review URL: https://codereview.chromium.org/1844743002 Cr-Commit-Position: refs/heads/master@{#384032} [modify] https://crrev.com/a05d65bcb2ac1339a99beaa22cf2e9ede5ff7d72/chrome/test/chromedriver/run_buildbot_steps.py [modify] https://crrev.com/a05d65bcb2ac1339a99beaa22cf2e9ede5ff7d72/chrome/test/chromedriver/util.py
,
Apr 13 2016
,
Apr 27 2016
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by d...@chromium.org
, Mar 28 2016