8330998: System.console() writes to stderr when stdout is redirected

[lahodaj]

Consider code like:

public class ConsoleTest {
    public static void main(String... args) {
        System.console().printf("Hello!");
    }
}

When run as:

$ java ConsoleTest.java >/dev/null

it prints Hello! to stderr, instead of to stdout (where it would be redirected).

The proposed fix is to simply force the use of stdout. Sadly, this cannot be done solely using JLine configuration, we actually need to change the JLine's code for that.

The most tricky part is a test. There are two sub-tests, one effectively testing a case where all of stdin/out/err are redirected, the other is attempting to test the case where stdin is attached to a terminal, while stdout is redirected. The second sub-test using a native functions to create a pty and to attach to it, and should run in a separate VM, as it leaves the VM attached to the terminal.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8330998: System.console() writes to stderr when stdout is redirected (Bug - P2)

Reviewers

• Naoto Sato (@naotoj - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/18996/head:pull/18996
$ git checkout pull/18996

Update a local copy of the PR:
$ git checkout pull/18996
$ git pull https://git.openjdk.org/jdk.git pull/18996/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 18996

View PR using the GUI difftool:
$ git pr show -t 18996

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/18996.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back jlahoda! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@lahodaj This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8330998: System.console() writes to stderr when stdout is redirected

Reviewed-by: naoto

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 30 new commits pushed to the master branch:

• 0a24dae: 8322043: HeapDumper should use parallel dump by default
• 62d5d1e: 8319598: SMFParser misinterprets interrupted running status
• 2a95cd4: 8331495: Limit BasicDirectoryModel/LoaderThreadCount.java to Windows only
• e833bfc: 8331222: Malformed text in the jpackage doc page
• 4f529f8: 8331427: Rename confusingly named ArraysSupport.signedHashCode
• 44dc850: 8331212: Error recovery for broken switch expressions could be improved
• b2fb5ea: 8331142: Add test for number of loader threads in BasicDirectoryModel
• 663acd2: 8330969: scalability issue with loaded JVMTI agent
• f215899: 8331393: AArch64: u32 _partial_subtype_ctr loaded/stored as 64
• b96b38c: 8318682: SA decoding of scalar replaced objects is broken
• ... and 20 more: https://git.openjdk.org/jdk/compare/151ef5d4d261c9fc740d3ccd64a70d3b9ccc1ab5...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk]

@lahodaj The following labels will be automatically applied to this pull request:

• core-libs
• kulla

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 02: Full - Incremental (76599ac9)
• 01: Full - Incremental (8a918e3f)
• 00: Full (9090e4c7)

[naotoj]

Looks good to me. Left some minor suggestions.
BTW, should we file an issue at the JLine project, not to redirect to stderr, or suggest a new config (sorry if it has already been taken care of)?

[naotoj]

@modules jdk.internal.le is needed, as the test relies on it.

[naotoj]

Could utilize ProcessTools test library to start the test jdk process (applies to the other location as well)

[ecki]

I think it's a feature when System.console() actually writes to the tty if stdout is redirected. After all it's not System.out. Of course the question is if it should write to stderr or /dev/tty like mechanism..

[archiecobbs]

Of course the question is if it should write to stderr or /dev/tty like mechanism..

I was wondering the same thing. My understanding of the definition of "console" is a bidirectional byte channel with a keyboard & screen on the other end. It has no concept of stdout vs. stderr. It just has a display (or in the old days, a printer).

The function of a "shell" is to intermediate between one or more executing programs and the console. It figures out how & when to actually display on the console any stuff written to stdout and/or stderr by one of the programs it has launched.

A program can also access its console (if any) directly by opening /dev/tty or whatever, thereby bypassing the shell.

So I would think writing to something called "System.console()" from Java (which is a program) would have nothing to do with Java's stderr or stdout, except for a possible downstream interleaving with what the shell may also be writing to the console at the same time.

[lahodaj]

Of course the question is if it should write to stderr or /dev/tty like mechanism..

I was wondering the same thing. My understanding of the definition of "console" is a bidirectional byte channel with a keyboard & screen on the other end. It has no concept of stdout vs. stderr. It just has a display (or in the old days, a printer).

The function of a "shell" is to intermediate between one or more executing programs and the console. It figures out how & when to actually display on the console any stuff written to stdout and/or stderr by one of the programs it has launched.

A program can also access its console (if any) directly by opening /dev/tty or whatever, thereby bypassing the shell.

So I would think writing to something called "System.console()" from Java (which is a program) would have nothing to do with Java's stderr or stdout, except for a possible downstream interleaving with what the shell may also be writing to the console at the same time.

I tried a few programs, like mc, top and htop. When I redirect the stdout for them, they still write into it (and there's obviously nothing on the terminal, and there are escape sequences in the target file of the redirect). The only program I know from the top of my head that (AFAIK) bypasses stdin/stdout and reaches directly to the controlling terminal is ssh when reading passwords (for quite obvious very special reasons).

I.e. not trying to be too smart about output, and simply using stdout as other programs do seems consistent, and most useful - the output can then be used in pipes, etc.

[archiecobbs]

I.e. not trying to be too smart about output, and simply using stdout as other programs do seems consistent, and most useful - the output can then be used in pipes, etc.

Totally reasonable. But the ssh example is telling - suppose for example someone wanted to implement something like ssh in Java. Would that person expect to be able to do what ssh does (accepting non-echoed passwords) by using System.console() ?

In other words, what exactly is System.console() supposed to represent? Maybe that question pinpoints the source of the ambiguity here. Of course, it will differ by O/S which adds to the challenge.

These questions are probably beyond the scope of this PR but you've got me curious :)

[lahodaj]

Looks good to me. Left some minor suggestions. BTW, should we file an issue at the JLine project, not to redirect to stderr, or suggest a new config (sorry if it has already been taken care of)?

The code to choose the output stream is quite elaborate, so I assume that is intentional. For new versions of JLine, the systemOutput(SystemOutput.SysOut) forces the use of stdout, however, so the patch to JLine itself will not be needed anymore after we upgrade.

[naotoj]

Looks good!

[lahodaj]

/integrate

[openjdk]

Going to push as commit f1509e0.
Since your change was applied there have been 90 commits pushed to the master branch:

• f2c4a41: 8328481: Implement JEP 476: Module Import Declarations (Preview)
• 9347bb7: 8330247: C2: CTW fail with assert(adr_t->is_known_instance_field()) failed: instance required
• b20fa7b: 8329982: compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/SimpleDebugInfoTest.java failed assert(oopDesc::is_oop_or_null(val)) failed: bad oop found
• c1a1645: 8331655: ClassFile API ClassCastException with verbose output of certain class files
• 36c9607: 8331591: sun.font.CharSequenceCodePointIterator is buggy and unused
• b33096f: 8295153: java/util/Base64/TestEncodingDecodingLength.java ran out of memory
• cf2c80e: 8331582: Incorrect default Console provider comment
• 77b7122: 8312104: Update java man pages to include new security category in -XshowSettings
• 87bb66c: 8331569: G1: Rename G1HRPrinter to G1HeapRegionPrinter
• 37c2469: 8331633: Use MIN2 in bound_minus_alignment
• ... and 80 more: https://git.openjdk.org/jdk/compare/151ef5d4d261c9fc740d3ccd64a70d3b9ccc1ab5...master

Your commit was automatically rebased without conflicts.

[openjdk]

@lahodaj Pushed as commit f1509e0.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.