On Tue, 6 Sep 2022 09:49:16 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:

> It's a well known behavior that overriding the`read(...)` method is 
> sufficient to implement subclass behavior. This will no longer be the case if 
> `transferTo(...)` no longer calls `this.read(...)` as it used to. There are 
> many undocumented behaviors that have been depended on over the year - and 
> though I agree with the sentiment that it's not a good idea to depend on 
> undocumented behavior, this is one that is long standing, and mostly 
> legitimate, especially for those classes that were coded before `transferTo` 
> was added to `InputStream`.
> 
> The issue here is that things would mostly work for those custom subclassed, 
> up to the point where some unsuspecting code might call `transferTo`. This 
> could lead to bugs that might become hard to diagnose, and of course would 
> only be seen at runtime. As a point of comparison, look at what [JEP 
> 425](https://openjdk.org/jeps/425) did to change the locking in 
> `BufferedInputStream` (see how the lock object is used/initialized).
> 
> How many classes of `BufferedInputStream` are there in the wild, outside of 
> the JDK? How many of them have custom behavior implemented in `read`, that 
> would be broken if `transferTo` no longer called `this.read`?
> 
> If you're changing an observable behavior, you will need a CSR, you will need 
> to evaluate the compatibility risk and justify it, you will need to write a 
> Release Note to warn about the compatibility risk, so that custom 
> implementations that have such a dependency can be changed to also override 
> `transferTo`.
> 
> The main question is whether the advantage of changing the behavior outweigh 
> the risk of regression it might cause in subclasses. Such might be the case - 
> or not. The middle ground - and safer path - is to override the behaviour 
> only for the base class and those subclasses that do not depend on the 
> default behavior as was done by JEP 425...

I understand your claim but disagree with the conclusion. You could impose it 
to *any* behavior change of *any* class -- this is no special case here. I do 
not see that we need a CSR as I do not change the *documented* behavior 
(neither the JavaDoc nor the Spec). Also I need to ask back: Do you *really* 
assume there are such lots of people using `transferTo` with failing subclasses 
of BIS? I doubt that, actually. You try to fix things possible not broken at 
all. We should concentrate on facts, not on assumptions, IMHO.

Having said that, I leave it up to @AlanBateman to decide, as I have no strong 
feelings about the issue you raised.

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

PR: https://git.openjdk.org/jdk/pull/6935

Reply via email to