[8u] RFR: Backport of 8067796 (process) Process.waitFor(timeout, unit) doesn't throw NPE if timeout is less than, or equal to zero when unit == null

[Sergey Nazarkin]

Could anyone help review this patch? 

Thanks.		

/Sergey

> On Apr 5, 2020, at 22:44, Sergey Nazarkin <snazarkin at azul.com> wrote:
> 
> Thanks, Andrew. In simpler case 8037866(Replace the Fun class in tests with lambdas)  only misses a part of MOAT.java that was introduced by 8014066(ArrayList#removeRange):
> 
> $cat test/java/util/Collection/MOAT.java.rej
> --- MOAT.java
> +++ MOAT.java
> @@ -728,8 +722,8 @@
>         l.listIterator(0);
>         l.listIterator(l.size());
>         THROWS(IndexOutOfBoundsException.class,
> -               new Fun(){void f(){l.listIterator(-1);}},
> -               new Fun(){void f(){l.listIterator(l.size() + 1);}});
> +               () -> l.listIterator(-1),
> +               () -> l.listIterator(l.size() + 1));
> 
>         if (l instanceof AbstractList) {
>             try {
> 
> Final patchset would be
> 
> 1) http://cr.openjdk.java.net/~snazarki/webrev/8037866/
> 2) http://cr.openjdk.java.net/~snazarki/webrev/8067796/
> 
> 
> 
> Sergey Nazarkin
> 
> 
> 
> 
>> On Apr 1, 2020, at 18:14, Andrew Haley <aph at redhat.com> wrote:
>> 
>> On 4/1/20 3:30 PM, Sergey Nazarkin wrote:
>>> I’ve tracked down patchset required for clean backport. Beside 8037866 [2] only 8014066 [1] is required.  I’m a bit worrying about 8014066, the changeset may affect existent applications behavior.
>>> 
>>> With 8014066 all subsequent patches applied cleanly except 8067796:
>>> - for non-windows OSes waitFor method is at UNIXProcess.java (not ProcessImpl.java)
>>> - windows version of ProcessImpl.java is placed at different path
>>> - Basic.java: bug list is different
>>> 
>>> If 8014066 is accepted for backport I’ll create separate RFRs.  In this case patch sequence should be [4] -> [5] -> [6]
>>> 
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8014066
>>> [2] https://bugs.openjdk.java.net/browse/JDK-8037866
>>> [3] https://bugs.openjdk.java.net/browse/JDK-8067796
>>> 
>>> [4] http://cr.openjdk.java.net/~snazarki/webrev/8014066/
>>> [5] http://cr.openjdk.java.net/~snazarki/webrev/8037866/
>>> [6] http://cr.openjdk.java.net/~snazarki/webrev/8067796/
>> 
>> You're right. This is too much for the initial patch, which is low
>> risk and changes only a few lines. It'd be OK if you find a way to do
>> it without changing ArrayList#removeRange. It wouldn't be a disaster
>> if you rewrote the test case to not require JDK-8037866.
>> 
>> -- 
>> Andrew Haley  (he/him)
>> Java Platform Lead Engineer
>> Red Hat UK Ltd. <https://www.redhat.com>
>> https://keybase.io/andrewhaley
>> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
>