thegrizzlylabs / sardine-android

A WebDAV library for Android
Apache License 2.0
355 stars 70 forks source link

Logs show a lot of connexions leaked #43

Open 3c71 opened 4 years ago

3c71 commented 4 years ago

It seems response bodies are never closed and I get many of those similar logs:

A connection to xxxxxxxxx was leaked. Did you forget to close a response body? 2020-03-21 12:39:07.457 28144-29092/xxxx W/OkHttp: java.lang.Throwable: response.body().close() 2020-03-21 12:39:07.457 28144-29092/xxxx W/OkHttp: at okhttp3.internal.platform.Platform.getStackTraceForCloseable(Platform.kt:134) 2020-03-21 12:39:07.457 28144-29092/xxxx W/OkHttp: at okhttp3.internal.connection.RealCall.callStart(RealCall.kt:166) 2020-03-21 12:39:07.457 28144-29092/xxxx W/OkHttp: at okhttp3.internal.connection.RealCall.execute(RealCall.kt:145)

I've started fixing those, and will post a PR if I get time to test.

However I have a question for you, which one would be best:

response.body().close(); // <-- This leads to a warning that body() might be null. or response.close(); // <-- This does an assert that'll throw an exception if body() is null.

I'd rather change the last close() to be safe no matter what. We don't want a crash or an exception if the query has been processed and something goes wrong after, do we? I don't and rather see things go smooth without having to manage exceptions everywhere (or the bare minimum), IMHO.