Skip to content

fix: avoid NPE when closing terminal with null masterOutput#1813

Merged
gnodet merged 1 commit into
masterfrom
ci-issue-1796
Apr 23, 2026
Merged

fix: avoid NPE when closing terminal with null masterOutput#1813
gnodet merged 1 commit into
masterfrom
ci-issue-1796

Conversation

@gnodet
Copy link
Copy Markdown
Member

@gnodet gnodet commented Apr 23, 2026

Fixes #1796

Summary

  • Add null guards for masterOutput in LineDisciplineTerminal.FilteringOutputStream.flush() and close() to prevent NPE when closing a terminal created with system(false) and no output stream
  • Also guard processOutputByte(), processInputByte(), and processInputBytes() for completeness
  • Add test verifying that closing an ExternalTerminal with null masterOutput does not throw

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Improved terminal stability by adding defensive null checks to output processing and stream closure operations, preventing potential exceptions when output streams are unavailable.
  • Tests

    • Added new test case to verify the terminal closes gracefully in edge-case scenarios.

When a LineDisciplineTerminal is created with a null masterOutput
(e.g. via TerminalBuilder.builder().system(false).build()), closing
the terminal triggers a NullPointerException in FilteringOutputStream's
flush() and close() methods. Add null guards for masterOutput in all
code paths.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d13d5855-1f29-4304-9fe1-4c0e9ef94df9

📥 Commits

Reviewing files that changed from the base of the PR and between eeb053e and 8c8975b.

📒 Files selected for processing (2)
  • terminal/src/main/java/org/jline/terminal/impl/LineDisciplineTerminal.java
  • terminal/src/test/java/org/jline/terminal/impl/NullOutputCloseTest.java

📝 Walkthrough

Walkthrough

Fixed a NullPointerException when closing a terminal with a null master output stream by adding null checks to guard flush and close operations in LineDisciplineTerminal and its FilteringOutputStream inner class. Includes a test verifying closure completes without exceptions.

Changes

Cohort / File(s) Summary
Null Safety Guards
terminal/src/main/java/org/jline/terminal/impl/LineDisciplineTerminal.java
Added null checks for masterOutput in input-driven flushes, output byte processing, and FilteringOutputStream's flush() and close() methods to prevent NPE during terminal closure when master output is null.
Test Coverage
terminal/src/test/java/org/jline/terminal/impl/NullOutputCloseTest.java
New JUnit 5 test that verifies ExternalTerminal.close() completes without throwing exceptions when master output is null.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A hop, a fix, a null to tame,
No more crashes in the closing game,
With guards in place, the stream runs free,
Now terminals close so happily! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately describes the main change: adding null guards to prevent NullPointerException when closing a terminal with null masterOutput.
Linked Issues check ✅ Passed The PR fully addresses issue #1796 by adding null guards in FilteringOutputStream.flush() and close() methods, plus guards in related methods, and includes a test verifying the fix works.
Out of Scope Changes check ✅ Passed All changes are directly within scope: null guards in LineDisciplineTerminal.java and a test in NullOutputCloseTest.java both address the linked issue requirement.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ci-issue-1796

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

@gnodet gnodet merged commit 9d07511 into master Apr 23, 2026
11 checks passed
@gnodet gnodet deleted the ci-issue-1796 branch April 23, 2026 17:07
gnodet added a commit that referenced this pull request Apr 23, 2026
The NPE when closing system(false) terminals was fixed in #1813. Update
IntegrationTest to use try-with-resources instead of leaking terminals,
and add a TerminalBuilder-level close test to NullOutputCloseTest.
gnodet added a commit that referenced this pull request Apr 27, 2026
…1813)

When a LineDisciplineTerminal is created with a null masterOutput
(e.g. via TerminalBuilder.builder().system(false).build()), closing
the terminal triggers a NullPointerException in FilteringOutputStream's
flush() and close() methods. Add null guards for masterOutput in all
code paths.
gnodet added a commit that referenced this pull request Apr 27, 2026
…1813)

When a LineDisciplineTerminal is created with a null masterOutput
(e.g. via TerminalBuilder.builder().system(false).build()), closing
the terminal triggers a NullPointerException in FilteringOutputStream's
flush() and close() methods. Add null guards for masterOutput in all
code paths.
gnodet added a commit that referenced this pull request Apr 27, 2026
…1813)

When a LineDisciplineTerminal is created with a null masterOutput
(e.g. via TerminalBuilder.builder().system(false).build()), closing
the terminal triggers a NullPointerException in FilteringOutputStream's
flush() and close() methods. Add null guards for masterOutput in all
code paths.
@gnodet gnodet added the fix Bug fix label May 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: NPE closing non-system terminal

1 participant