RFR: 8310530: PipedOutputStream.flush() accesses sink racily [v2]

[Daniel Fuchs]

On Wed, 21 Jun 2023 14:27:26 GMT, Chen Liang <liach at openjdk.org> wrote:

>> src/java.base/share/classes/java/io/PipedOutputStream.java line 166:
>> 
>>> 164:     @Override
>>> 165:     public synchronized void flush() throws IOException {
>>> 166:         PipedInputStream sink = this.sink;
>> 
>> Suggestion:
>> 
>>         var sink = this.sink;
>> 
>> As seen in other methods.
>
> On second thought, this is probably not necessary; write to the sink field is in another synchronized method, and this method is synchronized already. Is the goal here to remove the synchronized on flush?

Good observation. Removing `synchronized` on flush might be a worthwhile goal but possible side effects (including on potential subclasses) should be carefully considered.
I support stashing `sink` in a local variable though, even if the pointer can't be concurrently modified, just to make it clear that we only have one volatile read.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14589#discussion_r1237152608