Skip to content

Fix NPE in TemporaryBuffer when writing to closed buffer#6

Merged
pstreef merged 1 commit intomainfrom
fix/temporary-buffer-npe
Dec 3, 2025
Merged

Fix NPE in TemporaryBuffer when writing to closed buffer#6
pstreef merged 1 commit intomainfrom
fix/temporary-buffer-npe

Conversation

@pstreef
Copy link
Contributor

@pstreef pstreef commented Dec 3, 2025

Problem

Large repository operations that exceed the buffer's in-core limit can trigger a NullPointerException in TemporaryBuffer.write():

java.lang.NullPointerException: Cannot invoke "java.util.ArrayList.size()" because "this.blocks" is null
    at org.openrewrite.jgit.util.TemporaryBuffer.last(TemporaryBuffer.java:352)
    at org.openrewrite.jgit.util.TemporaryBuffer.write(TemporaryBuffer.java:111)
    at org.openrewrite.jgit.transport.PacketLineOut.end(PacketLineOut.java:203)
    at org.openrewrite.jgit.transport.BasePackConnection.close(BasePackConnection.java:652)

This happens when:

  1. Buffer exceeds in-core limit → switchToOverflow() sets blocks = null
  2. Transport operation fails
  3. close() is called, setting overflow = null
  4. Error cleanup calls pckOut.end() which tries to write to the closed buffer
  5. NPE when accessing blocks.size()

The NPE masks the original transport error because it's thrown during cleanup.

Solution

Add defensive checks in write() methods to throw IOException instead of NPE when the buffer is in a closed state (both blocks and overflow are null).

Why this works

The key insight is in BasePackConnection.close():

try {
    if (outNeedsEnd) {
        pckOut.end();  // throws IOException now, not NPE
    }
} catch (IOException err) {
    // Ignore any close errors
}
  • NPE (RuntimeException): Not caught → bubbles up and masks the original error
  • IOException: Caught and ignored → close() completes → original error propagates

This allows the actual transport failure to be reported instead of the cryptic NPE.

When a TemporaryBuffer exceeds its in-core limit, switchToOverflow()
sets blocks=null. If close() is then called, overflow is also set to
null. Any subsequent write attempt caused a NullPointerException
when accessing blocks.size().

This occurs during error cleanup in HTTP transport when
BasePackConnection.close() tries to write an end marker via
PacketLineOut.end() after the buffer has already been closed.

Add defensive checks in write() methods to throw IOException instead
of NPE when the buffer is closed. This allows the original transport
error to propagate correctly.
@github-project-automation github-project-automation bot moved this from In Progress to Ready to Review in OpenRewrite Dec 3, 2025
@pstreef pstreef merged commit 24895fb into main Dec 3, 2025
@pstreef pstreef deleted the fix/temporary-buffer-npe branch December 3, 2025 09:36
@github-project-automation github-project-automation bot moved this from Ready to Review to Done in OpenRewrite Dec 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants