Makes requests uncancelable when running on Java lower than 16 where cancellation is not supported

[vladislavsheludchenkov]

Closes #1188

The original issue this PR was aiming to solve was fixed in cats-effect, but only when running Java 16 and above. Cancellation is not supported below Java 16, and attempting to do so still leaks connections, so this PR makes the requests uncancelable instead of silently leaking them.

Original PR description before cats-effect fix:

Turns out cancellation of a completable future is different here based on mayInterruptIfRunning argument: if false, it reports successful cancellation but allows the underlying request to continue and leak resources since we aren't going to subscribe to its body; if true, it actually cancels the underlying request

It's not documented anywhere publicly, but mentioned in internal javadocs:

https://github.com/openjdk/jdk21/blob/master/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java#L272

https://github.com/openjdk/jdk21/blob/master/src/java.net.http/share/classes/jdk/internal/net/http/common/Cancelable.java#L38

CompletableFutureTerminationTest has been modified to fail if cancelled using mayInterruptIfRunning equal to false

Since the value for mayInterruptIfRunning is hardcoded in cats-effect, I had to copy the whole fromCompletableFuture. This isn't intended to be a permanent solution, my next steps will be creating a pull request to cats-effect to allow parametrization in that method and then using it in this repo once it's released, but opening this PR first to (1) get your feedback on if it's the right solution and (2) fix it as soon as possible since it looks like a major issue. Of course, let me know if you don't think it's the correct course of action and you prefer me starting with a PR to cats-effect

[armanbilge]

Thanks! This was already fixed in Cats Effect, I think we just need to release.

• Set mayInterruptIfRunning = true when canceling CompletableFuture typelevel/cats-effect#4375

[vladislavsheludchenkov]

Perfect! Was looking at series/3.x branch and missed this change. Do you know if a new patch version of cats effect is getting released anytime soon? If yes, feel free to close this PR. Having a test that can detect this leak is probably still useful, but I can open a new PR after the bump

[Prillan]

@armanbilge Would it be possible to have a 3.6.2 release for Cats Effect soon to fix this? We have an ugly workaround to prevent cancellation of requests in the most impacted places, but you never know when it's going to start leaking connections elsewhere

[hamnis]

If the 3.6.2 is far off, i am not against merging this.

[vladislavsheludchenkov]

Right, the cancellation test is failing on Java 11. That makes sense, as cancellation was implemented in version 16. Not sure how we should handle this. Branch in runtime on java version and make our effect uncancellable if it’s lower than 16?

[hamnis]

I think we have some JDK version checking here somewhere else, so that would not be unreasonable.

https://github.com/http4s/http4s-jdk-http-client/blob/series/0.10/core/src/main/scala/org/http4s/jdkhttpclient/JdkHttpClient.scala#L277

[vladislavsheludchenkov]

Rebased and updated this PR to specifically target the issue with JDK < 16, since it's already fixed now when running JDK >= 16