RFR: 8305457: Implement java.io.IO [v7]

[Pavel Rappo]

On Fri, 10 May 2024 07:45:02 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Pavel Rappo has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix System.console().readln(null) in jshell
>>   
>>   Without it, jshell hangs on me. Will think of a test.
>
> src/java.base/share/classes/jdk/internal/io/JdkConsoleImpl.java line 61:
> 
>> 59:     @Override
>> 60:     public JdkConsole println(Object obj) {
>> 61:         pw.println(obj);
> 
> Are these `println(...)` and `print(...)` methods intentionally not using a `writeLock` unlike the `readln(...)` and `readLine(...)` methods which do use (read and write) locks?

When implementing, I asked myself if I must use any of those monitors and decided that I don't have to. My reasoning is below.

`readLock`:

- is used inside the object that `Reader reader` is initialised with, and

- it also guards fields such as `char[] rcb`, `boolean restoreEcho` and `boolean shutdownHookInstalled`.

Since `println` and `print` don't call methods on `reader` or access the above fields, they don't require `readLock`.

`writeLock`:

- is used inside objects that `Writer out` and `PrintWriter pw` are initialised with, and
- also in compound operations that first write and then immediately read. (I assume, it's so that no concurrent write could sneak in between writing and reading parts of such a compound.)

`println` or `print` don't call methods on `out` and certainly don't do any write-read compounds. That said, they call methods on `pw`. But `pw` uses `writeLock` internally. So in that sense we're covered. 

One potential concern is a write-write compound in `print`:


    @Override
    public JdkConsole print(Object obj) {
        pw.print(obj);
        pw.flush(); // automatic flushing does not cover print
        return this;
    }


I'm saying write-_write_, not write-_flush_, because as far as synchronisation is concerned,  `pw.flush()` should behave the same as `pw.print("")`.

While not using `writeLock` is not strictly correct, I believe the potential execution interleavings with other writes are benign. What's the worst that could happen? You flush more than you expected? Big deal!

Since we exhausted all the reasons to use `writeLock`, I don't think we need one.



Naoto has already reviewed this PR with only minor comments. While that increases my confidence in that the implementation is correct, it doesn't hurt to request re-review of this specific part: @naotoj, do you think I should use any of those monitors?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/19112#discussion_r1596861708