sadr0b0t / yashlang

PeerTube and YouTube player for Android with local playlists and whitelisted recommendations
GNU General Public License v3.0
51 stars 3 forks source link

Добавить таймаут на подключение и чтение во все HttpConnection #132

Closed sadr0b0t closed 2 years ago

sadr0b0t commented 2 years ago

В HttpConnection по умолчанию не выставлен таймаут на чтение, поэтому затянувшаяся процедура чтения без явной ошибки может длиться бесконечно. Таймаут на подключение тоже не выставлен, но затянувшееся подключение может оборвать операционная система.

В некоторых ситуациях это может привести к тому, что, например, процедура закачки файла зависнет на бесконечно.

проблема https://github.com/sadr0b0t/yashlang/issues/8#issuecomment-1185243385 воспроизвести https://github.com/sadr0b0t/yashlang/issues/8#issuecomment-1185873888 решение https://github.com/sadr0b0t/yashlang/issues/8#issuecomment-1186151657

Пример выставления настроек подключения в NewPipe: https://github.com/TeamNewPipe/NewPipe/blob/3901ffca17cfd86bcee17ca2d9410b7fd94b0172/app/src/main/java/org/schabi/newpipe/player/datasource/YoutubeHttpDataSource.java

        final HttpURLConnection httpURLConnection = openConnection(new URL(requestUrl));
        httpURLConnection.setConnectTimeout(connectTimeoutMillis);
        httpURLConnection.setReadTimeout(readTimeoutMillis);

Значения таймаутов берутся из библиотеки ExoPlayer

import static com.google.android.exoplayer2.upstream.DefaultHttpDataSource.DEFAULT_CONNECT_TIMEOUT_MILLIS;
import static com.google.android.exoplayer2.upstream.DefaultHttpDataSource.DEFAULT_READ_TIMEOUT_MILLIS;

Пояснение по таймаутам

https://stackoverflow.com/questions/45199702/httpurlconnection-timeout-defaults https://docs.oracle.com/javase/7/docs/api/java/net/URLConnection.html#setReadTimeout(int)

Appears the "default" timeouts for HttpURLConnection are zero which means "no timeout." Unfortunately, in my experience, it appears using these defaults can lead to an unstable state, depending on what happens with your connection to the server. If you use an HttpURLConnection and don't explicitly set (at least read) timeouts, your connection can get into a permanent stale state. By default. So always set setReadTimeout to "something" or you might orphan connections (and possibly threads depending on how your app runs). It appears from trial and error that calling setConnectTimeout isn't required because the socket itself seems to have like a 2 minute "connect timeout" built in (at least in OS X). You may also be able to set a "global default" for the timeouts by adjusting system properties. Fix/prognosis: always set a readTimeout (even if very large), or use a different client that lets you set SO_KEEPALIVE. The default without these result in threads hanging "forever" without it (when/if they do a read), or on stale sockets sticking around forever...

Здесь еще предлагает вариант с опцией KEEP_ALIVE (можно при случае отдельно посмотреть о чем он)

Есть подозрение, что эта история могла генерировать ситуации с неосвобождаемыми ресурсами и глюками приложения, которые сложно зафиксировать, т.к. иконки роликов и плейлистов загружаются в большом количестве при прокрутке списков в фоновых потоках. В случае ошибки приложение никак об это не сообщает (и не должно) - просто вместо иконки видео остаётся заглушка, а это можно списать, например, на тормоза интернета.

Нужно выставить эти таймауты и на чтение и на подключение и не только здесь, а везде (как минимум при загрузке иконок роликов и плейлистов), значения взять, как и в NewPipe, из ExoPlayer.

sadr0b0t commented 2 years ago

https://github.com/sadr0b0t/yashlang/commit/395053c30d2e2b0d15488fcdc8f4bd699ad0747d