RFR: 8265261: java/nio/file/Files/InterruptCopy.java fails with java.lang.RuntimeException: Copy was not interrupted [v2]

[Mark Sheppard]

On Thu, 19 Aug 2021 15:32:15 GMT, Brian Burkhalter <bpb at openjdk.org> wrote:

>> test/jdk/java/nio/file/Files/InterruptCopy.java line 144:
>> 
>>> 142:             else {
>>> 143:                 result.get();
>>> 144:                 throw new RuntimeException("Copy was not cancelled");
>> 
>> Should `DURATION_MAX_IN_MS` be taken into account here too to decide whether to throw or not?
>
> I don't know. It seems like cancellation ought to be a bit more predictable than interruption. I am not sure how `DURATION_MAX_IN_MS` was determined. If there were no "hiccups" in the threads starting, then with the correct timing I am not sure that such a threshold is even needed. The execution time for the copy that I have observed is between 150ms (macOS) and 400ms (Windows) which is a lot less than 5s, so there would have to be some slowdown for this threshold even to be reached.

Not wishing to labour the discussion on the change, which will in the main deal with the issue, but is it not the case with the current set of Intermittent failures that there is this slowdown, i.e. the copy is taking longer than the threshold of 5 seconds hence the RuntimeException with Copy was not Interrupted?

As such,

if (duration > DURATION_MAX_IN_MS)
                    throw new RuntimeException("Copy was not interrupted");

is triggering a failure. 
You would expect if a typical copy duration is 400msecs that if the duration is taking a lot longer that the interrupt has a greater probability to happen.

the rationale for this logic is not clear - so if the duration is less than the copy threshold then test proceeds as if an interrupt has occurred?

the success and failure semantics for the test are not absolute. As such, maybe there's an opportunity to be more exact with test semantics e.g. retrying to interrupt the copy if it didn't happen. 

as an aside it would appear that invoking Thread.interrupted(); in the main thread seems redundant ?

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

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