Skip to content

Resource Leaks in processing/app/Util.java #1432

@SalmaneKhalili

Description

@SalmaneKhalili

Debugging:

Hello, I was looking around the codebase for the sake of getting familiar with, where i noticed a couple problematic resource usage implementations that do not guarantee closure with either finally or encompassing with try ().
In multiple instances, if n exception occurs (disk full, corrupt ZIP, network timeout, etc.) file handles remain open. I believe on Windows this prevents subsequent deletion and on any OS it can exhaust the system’s file‑descriptor limit after repeated use.

Examples:

  static public void unzip(File zipFile, File dest) throws IOException {
    FileInputStream fis = new FileInputStream(zipFile);
    CheckedInputStream checksum = new CheckedInputStream(fis, new Adler32());
    ZipInputStream zis = new ZipInputStream(new BufferedInputStream(checksum));
    ZipEntry entry;
    while ((entry = zis.getNextEntry()) != null) {
      final String name = entry.getName();
      if (!name.startsWith(("__MACOSX"))) {
        File currentFile = new File(dest, name);
        if (entry.isDirectory()) {
          currentFile.mkdirs();
        } else {
          File parentDir = currentFile.getParentFile();
          // Sometimes the directory entries aren't already created
          if (!parentDir.exists()) {
            parentDir.mkdirs();
          }
          currentFile.createNewFile();
          unzipEntry(zis, currentFile);
        }
      }
    }
  }

ZipInputStream, CheckedInputStream, and FileInputStream are never closed. i think this may also contribute to update issues if there are any, as any crash may lock the temporary zip file, preventing its deletion.

A simple solution would be:

public static void unzip(File zipFile, File dest) throws IOException {
  try (ZipInputStream zis = new ZipInputStream( new BufferedInputStream( new CheckedInputStream( new FileInputStream(zipFile), new Adler32())))) {
    ZipEntry entry;
    while ((entry = zis.getNextEntry()) != null) {
      //rest of code
    }
  }
}

this way all three underlying streams are closed automatically, even on exception.
This pattern repeats itself in other methods in the same file, some of which are: loadBytesRaw(File), saveFile(String, File) , copyFile(File, File).

Willing to work on this?

If i may, i'd be more than happy to be assigned to this issue as my first contribution to this codebase!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions