RFR: 8305457: Implement java.io.IO [v7]
Naoto Sato
naoto at openjdk.org
Fri May 10 16:44:12 UTC 2024
On Fri, 10 May 2024 14:54:10 GMT, Pavel Rappo <prappo at openjdk.org> wrote:
>> 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?
I think your investigation is correct. As to the write-write case, there already is the same pattern in (`formatter` basically utilizes `pw` underneath)
public JdkConsole format(String fmt, Object ... args) {
formatter.format(fmt, args).flush();
return this;
}
So I think it is acceptable.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19112#discussion_r1596976499
More information about the core-libs-dev
mailing list