Http2 Connection becomes stalled after interrupt during call.execute

[yschimke]

Reproduction in https://github.com/square/okhttp/pull/7014/files

With 1 Good thread - downloads work.

With 1 Good thread + 9 Cancelling (interrupt) threads - Download gets stuck, and other threads get SocketTimeout on following request

With 1 Good thread + 9 cancel + Interceptor setting noNewExchanges - Overall systems stays healthy, but download stays stuck.

[yschimke]

Relates to #3146

[yschimke]

It looks like this is meant to be covered

      val dataStream = getStream(streamId)
      if (dataStream == null) {
        writeSynResetLater(streamId, ErrorCode.PROTOCOL_ERROR)
        updateConnectionFlowControl(length.toLong())
        source.skip(length.toLong())
        return
      }

[yschimke]

It seems like it could be related to interrupts vs close.

This appears to fix the MediaTest.

          try {
            response.body!!.source().skip(8000001)
          } catch (ioe: InterruptedIOException) {
            response.close()
          }

I had thought that an interrupt was meant to cancel the call, but it doesn't seem like it does.

[yschimke]

I assume it's the leaked responses that don't get closed, but without being consumed are effectively blocking the entire connection.

[yschimke]

Also tangentially relevant https://github.com/coil-kt/coil/blob/main/CHANGELOG.md#200-alpha01---october-11-2021

Important: Coil now has its own disk cache implementation and no longer relies on OkHttp for disk caching.

This change was made to: Better support thread interruption while decoding images. This improves performance when image requests are started and stopped in quick succession.

You should not use OkHttp's Cache with Coil 2.0 as it can be corrupted if it's interrupted while writing to it.

[yschimke]

Gaaahhh!!!

The fix above calling response.close() only works after the response has been returned. If the interrupt happens during call.execute, it still get's into a bad state.

[yschimke]

I guess we can't even know the state of the Connection, maybe partially written output, so we'd need to close the whole connection. As I can do that externally to fix this, no point adding here. I guess the optimal change is to use enqueue instead of execute, avoid this failure condition, and deal with the interrupt later during reading the response.

Exception in thread "Thread-11" java.io.InterruptedIOException: interrupted
	at okio.Timeout.throwIfReached(Timeout.kt:98)
	at okio.OutputStreamSink.write(JvmOkio.kt:54)
	at okio.AsyncTimeout$sink$1.write(AsyncTimeout.kt:99)
	at okio.RealBufferedSink.flush(RealBufferedSink.kt:267)
	at okhttp3.internal.http2.Http2Writer.flush(Http2Writer.kt:120)
	at okhttp3.internal.http2.Http2Connection.newStream(Http2Connection.kt:268)
	at okhttp3.internal.http2.Http2Connection.newStream(Http2Connection.kt:225)
	at okhttp3.internal.http2.Http2ExchangeCodec.writeRequestHeaders(Http2ExchangeCodec.kt:76)
	at okhttp3.internal.connection.Exchange.writeRequestHeaders(Exchange.kt:63)
	at okhttp3.internal.http.CallServerInterceptor.intercept(CallServerInterceptor.kt:41)
	at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109)```

[yschimke]

Added some simple flags to the repro https://github.com/square/okhttp/pull/7014/files

  // Whether to interrupt the execute call or the body read.
  val interruptEarlyExecute = true

  // if true, then after an interrupt exception, the connection will be closed
  // thread 1 will recover if stuck on the next request after a SocketTimeoutException
  val repairConnection = false

  val detailedLogging = false

[yschimke]

I think this is covered by

1. Use Call.cancel instead of interrupts and Canceling and Interrupting doc #7185
2. the flow control fixes in Fix for stalled streams #7801