Skip to content

File handle in SpringBootJoranConfigurator should be closed - #51385

Open
dlwldn30 wants to merge 1623 commits into
spring-projects:4.0.xfrom
Goatshave:close-aot-file-content-stream
Open

File handle in SpringBootJoranConfigurator should be closed#51385
dlwldn30 wants to merge 1623 commits into
spring-projects:4.0.xfrom
Goatshave:close-aot-file-content-stream

Conversation

@dlwldn30

Copy link
Copy Markdown

What Problem This Solves

SpringBootJoranConfigurator.RequireNewOrMatchingContentFileHandler compares
the logging resources it is about to write against what is already there:

InputStreamSource content = file.getContent();
Assert.state(content != null, "Unable to get file content");
byte[] existingContent = content.getInputStream().readAllBytes();

InputStream.readAllBytes() is documented as "This method does not close the
input stream", and the stream is never assigned to anything, so nothing can
close it afterwards.

During AOT processing the handler is a FileSystemGeneratedFiles.FileSystemFileHandler,
whose content supplier returns new FileSystemResource(path), so this is a real
file handle rather than an in-memory resource. The handler is used for both
generated files (MODEL_RESOURCE_LOCATION and RESOURCE_LOCATION), and
file.exists() is true whenever more than one application context contributes
the same resource.

FileSystemGeneratedFiles itself already uses try-with-resources when it
consumes an InputStreamSource, so this is inconsistent with the code on the
other side of the same interface.

Evidence

Red — fix reverted, test present:

./gradlew :core:spring-boot:test --tests '*SpringBootJoranConfiguratorTests'

SpringBootJoranConfiguratorTests > aotContributionClosesExistingFileContent() FAILED
23 tests completed, 1 failed

java.lang.AssertionError:
Expecting all elements of:
  [AtomicBoolean(false)]
to match given predicate but this element did not:
  AtomicBoolean(false)

The failure is on the close assertion only. The IllegalStateException and
non-empty assertions before it pass, so the handler was invoked and the
comparison ran; only the stream was left open.

Green — same command with the fix applied:

BUILD SUCCESSFUL

Summary

Read the existing content inside a try-with-resources block. InputStream was
already imported, so the production change is three lines.

The test supplies its own GeneratedFiles and a FileHandler whose content
records when its stream is closed, then drives the AOT contribution through
applyTo.

Related

Same leak class as prior cleanups, all merged:

No open PR touches SpringBootJoranConfigurator, no closed PR in the last 200
has, and I found no existing issue for this. The only commits to this file are
the Spring Framework 7.0.6 snapshot upgrade, the nullability annotations and
the directory restructure, none of which touched this method.

Test plan

  • Red: aotContributionClosesExistingFileContent fails without the
    production change
  • Green: it passes with the fix
  • :core:spring-boot:test — 3829 tests, 0 failures, 0 errors, 14 skipped
  • checkFormatMain, checkFormatTest, checkstyleMain, checkstyleTest

Contributed on behalf of Goatshave.

snicoll and others added 30 commits July 21, 2026 14:51
See spring-projectsgh-50799

Signed-off-by: Ahmed El amraouiyine <amraouiyine@gmail.com>
* spring-projectsgh-50791-grpc-health-overall:
  Polish "Honor gRPC overall health setting"
  Honor gRPC overall health setting

Closes spring-projectsgh-50799
philwebb and others added 19 commits August 11, 2026 12:34
See spring-projectsgh-51348

Signed-off-by: Tran Ngoc Nhan <ngocnhan.tran1996@gmail.com>
Closes spring-projectsgh-51348

* remove-code-tag:
  Polish "Remove code tag in property description"
  Remove code tag in property description
AppendableByteArray.reset() was only called from toByteArray(), so an
encode that failed part-way left its partial output in the thread-local
cached instance. The next value encoded on that thread was then prefixed
with it, which in structured logging corrupted the log event following a
failed one.

Reset the cached instance when it is handed out instead, so the buffer
is clean regardless of how the previous use ended. This also covers the
early return in toByteArray() for empty content, which returns without
resetting.

See spring-projectsgh-51156

Signed-off-by: Sumit Kumar Das <skdas5405@gmail.com>
Closes spring-projectsgh-51156

* fix-51154-appendable-byte-array-reset:
  Polish "Reset cached AppendableByteArray before it is reused"
  Reset cached AppendableByteArray before it is reused
See micrometer-metrics/micrometer#2297

Co-authored-by: Stéphane Nicoll <stephane.nicoll@broadcom.com>
RequireNewOrMatchingContentFileHandler reads the already generated file
through content.getInputStream().readAllBytes(). InputStream.readAllBytes
does not close the stream, and the stream is never assigned, so it cannot
be closed at all.

During AOT processing the content is a FileSystemResource, so each
comparison leaks a file handle. FileSystemGeneratedFiles already uses
try-with-resources when it consumes an InputStreamSource.

Read the existing content inside a try-with-resources block.

See spring-projectsgh-51385

Signed-off-by: dlwldn30 <dlwldn30@naver.com>
@dlwldn30
dlwldn30 force-pushed the close-aot-file-content-stream branch from 04e3c5b to 16f5fff Compare August 17, 2026 05:04
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 17, 2026
@snicoll snicoll changed the title Existing content stream is not closed when writing AOT logging resources File content in SpringBootJoranConfigurator should be closed Aug 20, 2026
@snicoll snicoll changed the title File content in SpringBootJoranConfigurator should be closed File handle in SpringBootJoranConfigurator should be closed Aug 20, 2026
@snicoll snicoll added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 20, 2026
@snicoll snicoll self-assigned this Aug 20, 2026
@snicoll snicoll added this to the 4.0.9 milestone Aug 20, 2026
@snicoll
snicoll changed the base branch from main to 4.0.x August 20, 2026 07:30
@snicoll

snicoll commented Aug 20, 2026

Copy link
Copy Markdown
Member

Thanks for this PR. I checked out your branch locally to push a few minor tweaks and help get this over the finish line, but my push was rejected with a 403 error.

It looks like the "Allow edits from maintainers" option might be unchecked. You need to enable that. You can find the checkbox on the right-hand sidebar of this PR page, right at the bottom. Let me know once it's checked so I can push those updates. Also, please do not unched it going forward and enable it on the other PR you've submitted.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Aug 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: waiting-for-feedback We need additional information before we can continue type: bug A general bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.