RFR: 8u: 8076475: Misuses of strncpy/strncat

[Andrew Dinn]

Hi Andrew(s)

On 25/05/2020 07:16, Andrew Hughes wrote:
> 
> 
> On 03/04/2020 10:18, Andrew Haley wrote:
  . . .
>> Here is my reasoning. The need for stability and the minimization of
>> churn increases over time. People use JDK 8 because they want that
>> stability, and every patch carries some risk. So, ecause it is now a
>> mature piece of software, patches should be minimal where they
>> reasonably can. The practice of, by default, pulling in dependencies
>> from later releases is too risky: they might well be broken in ways
>> that the old version wasn't, and they may depend on other changes in
>> the recent code base.
>>
>> Now, that doesn't mean that such dependent changesets must *never* be
>> pulled in, especially when they are needed by several backports. But
>> if dependency changesets substantially multiply the size of a change,
>> as they do in this case, this should not be done.
>>
>> I am aware that this does not ease the life of the maintainers, but
>> that isn't the goal. Stability is.
> 
> It's not about making life easier for maintainers; reviewing and
> approving more patches means the opposite. My concern is that the more
> 8u diverges from later versions, and uses its own bespoke version of
> fixes, the greater the knowledge and experience needed for someone to
> work on it. We're already struggling to keep up as it is.

Well, that's one direction of travel. This is, however, a two way
street. The more jdk8u pulls in changes from later versions instead of
using its own bespoke version of fixes, the more likely it is that one
of those changes is going to be based on assumptions baked into the
upstream repo code that clash with assumptions baked into jdk8u repo
code. Understanding why something is breaking the jdk8u rules and how
that can be squared with what the the rest of the jdk8u code does is an
equal peril.

The danger is subtle errors where we fall foul of races, corner cases,
re-orderings of control or synchronization, different interpretation of
the same values in the same flags, counters or object fields. The
problems that may arise in this way cannot really easily be avoided by
adapting a patch as it is backported, even less so by pulling in more
patches. The obvious safeguards that are provided by getting the code to
compile and pass tests won't remove this sort of risk. The only way to
avoid those errors as you pull in ever more changes is to backport
everything so that the design decisions that constrain how jdk8u
operates have all been replaced with the upstream ones. However, that's
clearly not the destination we need to aim for.

None of these complex, subtle risks can be avoided via a simple,
mechanical process. All of them require the application of careful
judgement and a deep knowledge of the code bases involved. The more
releases there are between the original version of a patch and it's
application downstream the more thinking there needs to be. Given enough
distance (and with many patches we have more than enough distance even
between jdk11u and jdk8u) we can no longer afford to proceed by tweaking
an upstream patch, even less so by pulling in all the apparatus it
depends on. We have to think about the bug (or perf issue), think about
what that bug means in the context of jdk8u and then think about
designing a patch that fixes it from the point of view of jdk8u. The
original patch /may/ do the job, or /may/ guide us to the right fix.
However, if the upstream patch is dependent on a lot of other changes
not present in jdk8u then that's a flashing red light saying that this
patch won't do and that a custom patch is a better, safer bet.

> Each case has to be considered separately, according to risk and the
> changes involved. It's not a guarantee of stability to just not make
> changes. If it leads to the changes that are made to 8u being unique to
> that codebase, and not as widely tested as something that has been in
> later versions for years, we will introduce more 8u-specific bugs. In
> short, there is no easy answer and no one size fits all solution.

Yes, that is right. However, I agree with Andrew that we need to have a
very healthy mistrust of any patch that requires us to pull other
changes in its wake. Upstream fixes may legitimately depend on other
upstream fixes because ... well, they were already there and, when
added, one can assume they were known to be necessary and safe in that
upstream context. Downstream fixes should normally only presume the
presence of such a dependency if there is a compelling need to have that
dependency /in its own right/. If not then the default assumption has to
be that a downstream fix should be sought without relying on that
upstream dependency. Pulling in multiple patches simply because that was
a legitimate upstream choice is a very slippery and dangerous slope.

regards,


Andrew Dinn
-----------