Skip to content

JarFile is not closed when finding main class from archive#50949

Merged
snicoll merged 1 commit into
spring-projects:4.0.xfrom
SebTardif:fix/findmainclass-close-jarfile
Jul 16, 2026
Merged

JarFile is not closed when finding main class from archive#50949
snicoll merged 1 commit into
spring-projects:4.0.xfrom
SebTardif:fix/findmainclass-close-jarfile

Conversation

@SebTardif

Copy link
Copy Markdown
Contributor

What Problem This Solves

FindMainClass opens a JarFile when @classesRoot points at an archive and passes it to MainClassFinder.findSingleMainClass(JarFile, ...). That API does not close the jar; the caller owns the resource. The Ant task never closed it, leaving a file handle open for the rest of the JVM lifetime (or until GC finalization).

Almost every other new JarFile(...) call site in this repository already uses try-with-resources (Gradle/Maven plugins, loader tools, buildSrc checks, etc.).

Evidence

Red-green unit proof:

# Red (buggy code still present)
./gradlew :build-plugin:spring-boot-antlib:test --tests \
  org.springframework.boot.ant.FindMainClassTests.closesJarFileWhenSearchingArchive
FindMainClassTests > closesJarFileWhenSearchingArchive(Path) FAILED
    java.lang.AssertionError at FindMainClassTests.java:71
# assertion: getJarEntry after task.execute should throw IllegalStateException when jar is closed

# Green (try-with-resources applied)
BUILD SUCCESSFUL

Also verified: checkFormatMain/checkFormatTest, checkstyleMain/checkstyleTest.

Summary

Wrap the archive path in try-with-resources so the jar is closed after the main-class search returns.

Related

Test plan

  • Red: FindMainClassTests.closesJarFileWhenSearchingArchive fails without close
  • Green: same test passes with try-with-resources
  • Format + Checkstyle on main and test sources

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 14, 2026
@snicoll

snicoll commented Jul 14, 2026

Copy link
Copy Markdown
Member

@SebTardif thanks again. I have a feeling you open a new PR every time one gets closed. This back and forth is quite time consuming on our end. Can't we gather stream cleanup in an issue rather than creating so many different PRs? I am asking if you have piled up more changes.

@SebTardif

Copy link
Copy Markdown
Contributor Author

@snicoll No further stream/resource cleanup changes are lined up.

@SebTardif SebTardif closed this Jul 14, 2026
@snicoll

snicoll commented Jul 15, 2026

Copy link
Copy Markdown
Member

@SebTardif did you mean to close this one?

@SebTardif SebTardif reopened this Jul 15, 2026
@snicoll snicoll changed the title Close JarFile when finding main class from archive JarFile is not closed when finding main class from archive Jul 16, 2026
@snicoll snicoll self-assigned this Jul 16, 2026
@snicoll snicoll added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 16, 2026
@snicoll snicoll added this to the 4.0.8 milestone Jul 16, 2026
MainClassFinder.findSingleMainClass(JarFile, ...) does not close the
JarFile; the caller owns the resource. FindMainClass passed a newly
opened JarFile without closing it, unlike sibling call sites that use
try-with-resources.

See spring-projectsgh-50949

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
@snicoll
snicoll force-pushed the fix/findmainclass-close-jarfile branch from a7a5011 to c3153dc Compare July 16, 2026 09:08
@snicoll
snicoll changed the base branch from main to 4.0.x July 16, 2026 09:08
@snicoll
snicoll merged commit 15bb54a into spring-projects:4.0.x Jul 16, 2026
4 checks passed
@snicoll

snicoll commented Jul 16, 2026

Copy link
Copy Markdown
Member

@SebTardif while I appreciate taking the time to write a test for this, I don't think it is worth it considering the change and the additional librairies the test brings. Thanks again for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bug A general bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants