Temp files everywhere! Should there be fewer?

58 views
Skip to first unread message

Daniel Bratell

unread,
Jan 23, 2017, 9:32:08 AM1/23/17
to chromi...@chromium.org
When running Chromium unit tests they quite often create test data
dynamically with APIs such as CreateTemporaryFile(). These files seem to
quite often survive so you get more and more temporary files.

In itself, a lot of small temporary files is not a problem, but, as I just
encountered, the Win implementation uses ::GetTempFileName which can only
handle 64K files, and after that it starts failing and tests will fail.

Depending on the answer to the question in this mail, I would either
change to not use ::GetTempFileName or I would try to fix tests that don't
even try to delete their tests.

So my question: Should we bother with cleaning up temp files after
execution or should we make the temp file system more robust?

/Daniel

--
/* Opera Software, Linköping, Sweden: CET (UTC+1) */

Rachel Blum

unread,
Jan 23, 2017, 9:58:35 AM1/23/17
to Daniel Bratell, chromi...@chromium.org
My personal opinion - a test that leaves temp files around should be a failing test.  Let's hoist up Chromecast's ScopedTempFile and use that in tests.

I expect a lot of disagreement on that stance :)



--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:   http://groups.google.com/a/chromium.org/group/chromium-dev


Greg Thompson

unread,
Jan 23, 2017, 10:22:23 AM1/23/17
to gr...@chromium.org, Daniel Bratell, chromi...@chromium.org
+1 to what Rachel said. To help with this, tests should use ScopedTempDir, which self-deletes at destruction (aside from crashes). Tests should also use DELETE_ON_CLOSE where possible so that temp files are automagically deleted. This is resilient to crashes.

We have a helper thing for diddling in the Windows registry that automagically cleans stale data leftover from previous crashy tests. Maybe ScopedTempDir could have a mode for tests that does this, too? Who wants to write it!

Marc-Antoine Ruel

unread,
Jan 23, 2017, 4:31:39 PM1/23/17
to Greg Thompson, Rachel Blum, Daniel Bratell, chromi...@chromium.org
That can only work if there is continuous outside enforcement. Everything else is wishful thinking.

Each Swarming bot take many actions against leaking files:
- Remediation: It has its own list of globs of frequent leakers and clean them up after each test run.
- Protection: It deletes the work directory where the task ran in.
- Protection: It sets up a fake temporary directory that it injects to the task being run via simple environment variable.
- Enforcement: It hard fails a task where the parent process exits yet leaves child processes alive. It took years to get tests to not trigger this up...

Action items:
- Accountability: I filed crbug.com/579233 to fix python unnamed temporary directory usage.
- Enforcement: I filed crbug.com/684070 to track work on making Swarming tasks to hard fail on left over temporary files.

The first would be helpful to help diagnosis and would be relatively simple task to complete with the help of "sed".

The second is a quite good task if someone wants to learn a bit about the infrastructure but not too much. :)

Take on either bug and fire CLs away if interested. No need to ask.

M-A

Paweł Hajdan, Jr.

unread,
Jan 24, 2017, 7:20:01 AM1/24/17
to Marc-Antoine Ruel, Greg Thompson, Rachel Blum, Daniel Bratell, chromi...@chromium.org
+1 to enforce this in continuous integration

Another useful bug is https://bugs.chromium.org/p/chromium/issues/detail?id=361343 , where we could eventually make steps fail when they leak temporary files. As a start, deletion of the temporary files would be more robust on the infrastructure.

We could also consider adding similar logic to base/test/launcher (I could do that), such that each test process would get its own temporary directory, and then the launcher would check whether that directory is empty. Since sometimes we batch several tests in one process, the attribution wouldn't be ideal, but that could be a good start. We may need to have a way to whitelist current offenders while we push for fixing them. Let me know if this sounds like something we should look into further.

Paweł

--

Lei Zhang

unread,
Feb 21, 2017, 9:29:55 PM2/21/17
to bra...@opera.com, chromi...@chromium.org
Another vote for fixing tests that litter temp files.

This has been a problem for years and one of my pet peeves. We even
had worse offenders that wrote temp files in the source tree. I
vaguely remember one time long ago even a Linux bot choked because the
file system for /tmp had too many files and ran out of inodes.

On Mon, Jan 23, 2017 at 6:30 AM, Daniel Bratell <bra...@opera.com> wrote:

Wez

unread,
Feb 21, 2017, 9:36:53 PM2/21/17
to the...@chromium.org, bra...@opera.com, chromi...@chromium.org
+1 to this.  I had [no] fun tracking down crbug.com/691388 this weekend (it's particularly annoying because it leaks the file in the current-working-directory, not in temp).

Lei Zhang

unread,
Feb 21, 2017, 9:42:56 PM2/21/17
to Wez, bra...@opera.com, chromi...@chromium.org
:( My strategy is to run with --gtest_filter=A*, B*, and so on to
narrow it down and go from there. Sometimes peeking in the temp file
might give a clue as to what created it.

Charles Harrison

unread,
Feb 21, 2017, 9:43:30 PM2/21/17
to w...@chromium.org, Lei Zhang, bra...@opera.com, chromi...@chromium.org, wko...@chromium.org
FYI that ScopedTempDir still sometimes leaves files after test completion if it is (easily) misused on Windows. See crbug.com/546640.

You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.

Reply all
Reply to author
Forward
0 new messages