ChaoticTrials / SimpleBackups

A simple mod to create scheduled backups
https://modrinth.com/mod/simple-backups
Apache License 2.0
10 stars 6 forks source link

Infinite loop deleting files when there's a directory in the backup path #14

Closed Kakoen closed 1 year ago

Kakoen commented 1 year ago

Minecraft version

1.19.4

Simple Backups version

1.19.1-2.1.9

Forge version

43.2.6

The latest.log file

Not required

Issue description

I ran into an issue where this mod would end up in an infinite loop, taking 100% CPU.

The issue appears when a directory (or something else that cannot be deleted) is present in the simplebackups folder. E.g by extracting one of the zips.

The culprit is the following code:

    public void deleteFiles() {
        File backups = CommonConfig.getOutputPath().toFile();
        if (backups.isDirectory()) {
            File[] files = backups.listFiles();
            if (files != null && files.length >= CommonConfig.getBackupsToKeep()) {
                Arrays.sort(files, Comparator.comparingLong(File::lastModified));
                while (files.length >= CommonConfig.getBackupsToKeep()) {
                    boolean deleted = files[0].delete();
                    String name = files[0].getName();
                    if (deleted) {
                        LOGGER.info("Successfully deleted \"" + name + "\"");
                        files = Arrays.copyOfRange(files, 1, files.length);
                    }
                }
            }
        }
    }

Here, backups.listFiles() returns (contrary to what the name might suggest) both directories and files. So, files[0] is a directory, files[0].delete() returns false because a directory with contents cannot be deleted, and the files array is never modified, causing the while loop to repeat itself infinitely.

Also see the following Spark stacktrace simple-backups-cpu

Steps to reproduce

  1. Create a directory with contents in the simplebackups directory
  2. Make sure that directory is older than the created backups
  3. Watch you CPU spike when SimpleBackups tries to clean up old backups

Other information

No response