Target directories are not purged after build failures

[pietroalbini]

While there is code to purge target directories once a build finishes, if an error happens during the build that code is never executed, leaking disk space. We should make sure the target directory is always purged, otherwise we risk running out of disk space if a lot of crates fail to build (for example during today's outage).

[jyn514]

@pietroalbini I think this would be best addressed by making BuildDirectory having a drop impl that calls purge(). I can make a PR to that effect.

[pietroalbini]

That shouldn't be the default behavior, but I'd welcome a BuildDirectory::purge_on_drop(bool).

[jyn514]

As an alternative to the drop impl, we decided it would be more reliable to have a shared cleanup: AtomicBool. When the directory is created, cleanup is set to true to indicate there are resources that are being used. When the build finishes, the directory will be deleted and cleanup will be set to false. Whenever the daemon starts a new build, it will check cleanup, and if set to true, delete all build directories.

Rustwide will need a new .clean_build_directories() API to go with clean_caches().

[jyn514]

The advantage of this method is it works even if the thread is killed and does not run destructors.

[pietroalbini]

Hmm, wait, that's not what I was proposing during the triage meeting.

The builder thread currently starts each loop by checking if a crate is available to build, and if one is present it builds that crate. My suggestion is to add a check in every iteration of the loop that purges all the build directories and, if the disk usage is over a threshold, purges all the caches.

[jyn514]

Thanks for the correction, I keep calling it the wrong thing. What I mean is this bit of code:

docs.rs/src/utils/daemon.rs

Line 78 in bdaf33b

match doc_builder.get_new_crates() {

[syphar]

@Nemo157 what's your thought on this?

Currently we are not purging all build dirs on each iteration.

( we are purging all caches on toolchain change )

[syphar]

and while normal build failures would also lead to the release target directory being purged after the build,

there are some error situations where we use ?, where we wouldn't do this (apart from normal panics)