Skip to content

Conversation

@JinwooHwang
Copy link
Contributor

@JinwooHwang JinwooHwang commented Nov 16, 2025

Description

This PR fixes a missing banner issue, a history file migration issue and a critical bug where gfsh fails to start with a NullPointerException after the Spring Shell 1.x to 3.x migration.

Related JIRA: GEODE-10523

Problem

After migrating to Spring Shell 3.x, running gfsh results in:

Exception in thread "main" java.lang.NullPointerException: Cannot invoke "org.jline.reader.LineReader.readLine(String)" because "this.lineReader" is null
	at org.apache.geode.management.internal.cli.shell.Gfsh.promptLoop(Gfsh.java:622)
	at org.apache.geode.management.internal.cli.shell.Gfsh.run(Gfsh.java:282)
	at org.apache.geode.management.internal.cli.shell.Gfsh.main(Gfsh.java:235)

Additionally, two other issues were discovered:

  1. History file format incompatibility between JLine 2 and JLine 3
  2. Missing banner output in non-headless mode

Root Cause

Spring Shell 1.x automatically initialized the terminal and line reader during startup. Spring Shell 3.x requires explicit initialization of these components before use. The Gfsh.run() method was calling promptLoop() without initializing the terminal or creating the console reader first.

Solution

1. Terminal Initialization

Added initializeTerminal() method to explicitly create and initialize the JLine 3 Terminal:

private void initializeTerminal() throws IOException {
  if (terminal == null) {
    terminal = TerminalBuilder.builder()
        .system(true)
        .build();
  }
}

Modified run() method to initialize terminal and create console reader before entering prompt loop:

public int run(String... args) throws IOException {
  printBannerAndWelcome();
  initializeTerminal();
  createConsoleReader();
  
  String historyFileName = getHistoryFileName();
  // ...
  return promptLoop();
}

2. History File Migration

Implemented automatic migration from JLine 2 format (with // comments) to JLine 3 format (clean format):

In Gfsh.java:

private void migrateHistoryFileIfNeeded(Path historyPath) throws IOException {
  if (!Files.exists(historyPath)) {
    return;
  }
  
  List<String> lines = Files.readAllLines(historyPath);
  if (!lines.isEmpty() && (lines.get(0).startsWith("//") || lines.get(0).startsWith("#"))) {
    Path backupPath = Paths.get(historyPath.toString() + ".old");
    Files.move(historyPath, backupPath, StandardCopyOption.REPLACE_EXISTING);
    logger.info("Migrated old history file format. Old file backed up to: " + backupPath);
  }
}

In GfshHistory.java:

@Override
public void attach(LineReader reader) {
  try {
    super.attach(reader);
  } catch (IllegalArgumentException e) {
    if (e.getMessage() != null && e.getMessage().contains("Bad history file syntax")) {
      migrateOldHistoryFile();
      super.attach(reader);
    } else {
      throw e;
    }
  }
}

3. Banner Display Fix

Modified printAsInfo() to always print to stdout in addition to logging:

private void printAsInfo(String message) {
  // Always print to stdout for banner visibility
  println(message);
  
  // Also log when not headless
  if (!isHeadlessMode()) {
    logger.info(message);
  }
}

Files Modified

  • geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java

    • Added initializeTerminal() method
    • Added migrateHistoryFileIfNeeded() method
    • Modified run() method to initialize terminal before prompt loop
    • Modified printAsInfo() to output to stdout
    • Added imports: java.nio.file.Files, java.nio.file.Path
  • geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/jline/GfshHistory.java

    • Overrode attach() method with exception handling
    • Added migrateOldHistoryFile() method for format migration

Testing

Build Verification

./gradlew build

Result: BUILD SUCCESSFUL

Verified:

  • Banner displays correctly
  • No NullPointerException
  • History file automatically migrated on first run (old file backed up to .gfsh_history.old)
  • Interactive prompt works correctly

Example Output

    _________________________     __
   / _____/ ______/ ______/ /____/ /
  / /  __/ /___  /_____  / _____  / 
 / /__/ / ____/  _____/ / /    / /  
/______/_/      /______/_/    /_/    1.16.0-build.0

Monitor and Manage Apache Geode
gfsh>

Impact

  • Severity: Critical - gfsh completely non-functional without this fix
  • Scope: All users attempting to run gfsh after Spring Shell 3.x migration
  • Risk: Low - Changes are isolated to initialization code and backward compatible

Checklist

  • Code changes implemented and tested
  • Build passes successfully
  • Functional testing completed
  • No breaking changes to existing functionality
  • History migration is automatic and transparent to users
  • Old history files are safely backed up

For all changes, please confirm:

  • Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
  • Has your PR been rebased against the latest commit within the target branch (typically develop)?
  • Is your initial contribution a single, squashed commit?
  • Does gradlew build run cleanly?
  • Have you written or updated unit tests to verify your changes?
  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?

- Add terminal initialization before promptLoop()
- Implement history file migration from JLine 2 to JLine 3 format
- Fix banner display to stdout in non-headless mode

After migrating from Spring Shell 1.x to 3.x, terminal and lineReader
were not being initialized, causing NPE when gfsh tried to read input.
Also fixed incompatible history file format and missing banner output.
@JinwooHwang JinwooHwang requested a review from raboof November 16, 2025 18:05
@JinwooHwang JinwooHwang changed the title [GEODE-10523] NullPointerException in gfsh startup after Spring Shell 3.x migration [GEODE-10523] gfsh issues after Spring Shell 3.x migration Nov 16, 2025
@JinwooHwang JinwooHwang changed the title [GEODE-10523] gfsh issues after Spring Shell 3.x migration [GEODE-10523] gfsh issues after Spring Shell 3 migration Nov 16, 2025
@JinwooHwang JinwooHwang changed the title [GEODE-10523] gfsh issues after Spring Shell 3 migration [GEODE-10523] 2.0 RELEASE BLOCKER : gfsh issues after Spring Shell 3 migration Nov 16, 2025
- Revert printAsInfo() to use logger.info() in non-headless mode
  (matching pre-Jakarta migration behavior from commit 30cd678^)
- Move printBannerAndWelcome() after terminal initialization
- This ensures banner output is consistent with original behavior
@JinwooHwang
Copy link
Contributor Author

Hi @raboof, all checks have passed. Thank you for your support.

Copy link
Member

@raboof raboof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the change looks generally OK to me, though I don't have a setup on which to test it.

@JinwooHwang
Copy link
Contributor Author

Hi @raboof. Thank you so much for reviewing and approving! We’re ready to move forward with the merge.

@JinwooHwang JinwooHwang merged commit 490947a into apache:develop Nov 19, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants