On Sat, 16 Apr 2022 11:05:47 GMT, Jaikiran Pai <j...@openjdk.org> wrote:

>> Alan Bateman has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains five additional 
>> commits since the last revision:
>> 
>>  - Refresh
>>  - Refresh
>>  - Merge with jdk-19+18
>>  - Refresh
>>  - Initial push
>
> src/java.base/share/classes/java/io/FilterInputStream.java line 233:
> 
>> 231:      */
>> 232:     @Override
>> 233:     public void reset() throws IOException {
> 
> I suspect this change to remove synchronization from `mark()` and `reset()` 
> is intentional. The `FilterInputStream` class doesn't explain the thread 
> safety semantics, nor can I find commit history which would hint on why these 
> (and only these) methods were syncrhonized. But it looks like this 
> synchronization was there to serialize calls to `mark()` and `reset()` on the 
> same `FilterInputStream` from multiple threads. With this synchronization now 
> removed, subclasses or even direct usages of `FilterInputStream` would now be 
> impacted. Potentially outside of the JDK. Would that be a concern?

Yes, this is intentional but it would be great to split out this change and get 
them into the main line in advance. There is a JEP size task required to 
re-visit all of the locking in java.io. In the case of the input/output 
streams, most of it is not documented and there are several inconsistencies. In 
the case of FilterInputStream, it doesn't use synchronization for the other ops.

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

PR: https://git.openjdk.java.net/jdk/pull/8166

Reply via email to