From 80928e730c53729d147264a910fb327ba88be257 Mon Sep 17 00:00:00 2001
From: yschimke <yschimke@google.com>
Date: Thu, 5 May 2022 09:15:06 +0100
Subject: [PATCH] Workaround for OkHttp Interrupt issues.

Relates to https://github.com/square/okhttp/issues/3146. This was from https://github.com/androidx/media/pull/71.

There is a draft PR https://github.com/square/okhttp/pull/7185/files which documents OkHttp's ideal handling of cancellation including interrupts.

But a few key points

1) This is a target state, and OkHttp does not currently handle interrupts correctly.  In the past this has been identified, and the advice is to avoid interrupts on Http threads, see discussion on https://github.com/square/okhttp/issues/1902. Also an attempt at a fix here https://github.com/square/okhttp/pull/7023 which wasn't in a form to land.

2) Even with this fixed, it is likely to never be optimal, because of OkHttp sharing a socket connection for multiple inflight requests.

From https://github.com/square/okhttp/pull/7185

```
Thread.interrupt() is Clumsy
----------------------------

`Thread.interrupt()` is Java's built-in mechanism to cancel an in-flight `Thread`, regardless of
what work it's currently performing.

We recommend against using `Thread.interrupt()` with OkHttp because it may disrupt shared resources
including HTTP/2 connections and cache files. In particular, calling `Thread.interrupt()` may cause
unrelated threads' call to fail with an `IOException`.
```

This PR leaves the Loader/DataSource thread parked on a countdown latch, while this may seem wasteful and an additional context switch. However in practice the response isn't returned until the Http2Connection and Http2Reader have a response from the server and these means effectively parking in a `wait()` statement here https://github.com/square/okhttp/blob/9e039e94123defbfd5f11dc64ae146c46b7230eb/okhttp/src/jvmMain/kotlin/okhttp3/internal/http2/Http2Stream.kt#L140

PiperOrigin-RevId: 446652468
---
 RELEASENOTES.md                               |  1 +
 .../datasource/okhttp/OkHttpDataSource.java   | 38 ++++++++++++++++++-
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/RELEASENOTES.md b/RELEASENOTES.md
index 59cac86f2a..7c87e71a8e 100644
--- a/RELEASENOTES.md
+++ b/RELEASENOTES.md
@@ -89,6 +89,7 @@
 
 *   Data sources:
     *   Rename `DummyDataSource` to `PlaceHolderDataSource`.
+    *   Workaround OkHttp interrupt handling.
 *   Remove deprecated symbols:
     *   Remove `Player.Listener.onTracksChanged`. Use
         `Player.Listener.onTracksInfoChanged` instead.
diff --git a/libraries/datasource_okhttp/src/main/java/androidx/media3/datasource/okhttp/OkHttpDataSource.java b/libraries/datasource_okhttp/src/main/java/androidx/media3/datasource/okhttp/OkHttpDataSource.java
index 42ed422550..aaa3fe8766 100644
--- a/libraries/datasource_okhttp/src/main/java/androidx/media3/datasource/okhttp/OkHttpDataSource.java
+++ b/libraries/datasource_okhttp/src/main/java/androidx/media3/datasource/okhttp/OkHttpDataSource.java
@@ -32,10 +32,14 @@ import androidx.media3.datasource.DataSource;
 import androidx.media3.datasource.DataSourceException;
 import androidx.media3.datasource.DataSpec;
 import androidx.media3.datasource.HttpDataSource;
+import androidx.media3.datasource.HttpDataSource.HttpDataSourceException;
+import androidx.media3.datasource.HttpDataSource.InvalidContentTypeException;
+import androidx.media3.datasource.HttpDataSource.InvalidResponseCodeException;
 import androidx.media3.datasource.HttpUtil;
 import androidx.media3.datasource.TransferListener;
 import com.google.common.base.Predicate;
 import com.google.common.net.HttpHeaders;
+import com.google.common.util.concurrent.SettableFuture;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InterruptedIOException;
@@ -43,8 +47,10 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ExecutionException;
 import okhttp3.CacheControl;
 import okhttp3.Call;
+import okhttp3.Callback;
 import okhttp3.HttpUrl;
 import okhttp3.MediaType;
 import okhttp3.OkHttpClient;
@@ -299,8 +305,9 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource {
     Request request = makeRequest(dataSpec);
     Response response;
     ResponseBody responseBody;
+    Call call = callFactory.newCall(request);
     try {
-      this.response = callFactory.newCall(request).execute();
+      this.response = executeCall(call);
       response = this.response;
       responseBody = Assertions.checkNotNull(response.body());
       responseByteStream = responseBody.byteStream();
@@ -448,6 +455,35 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource {
     return builder.build();
   }
 
+  /**
+   * This method is an interrupt safe replacement of OkHttp Call.execute() which can get in bad
+   * states if interrupted while writing to the shared connection socket.
+   */
+  private Response executeCall(Call call) throws IOException {
+    SettableFuture<Response> future = SettableFuture.create();
+    call.enqueue(
+        new Callback() {
+          @Override
+          public void onFailure(Call call, IOException e) {
+            future.setException(e);
+          }
+
+          @Override
+          public void onResponse(Call call, Response response) {
+            future.set(response);
+          }
+        });
+
+    try {
+      return future.get();
+    } catch (InterruptedException e) {
+      call.cancel();
+      throw new InterruptedIOException();
+    } catch (ExecutionException ee) {
+      throw new IOException(ee);
+    }
+  }
+
   /**
    * Attempts to skip the specified number of bytes in full.
    *