Adding a timeout to the http connections to avoid indefinite hangs on socket.read0

[nosachamos]

I've been getting timeouts like these:

"Thread-14" #26347 prio=5 os_prio=0 tid=0x0000009f0277e000 nid=0x1338 runnable [0x0000009f0857e000]
java.lang.Thread.State: RUNNABLE
at java.net.SocketInputStream.socketRead0([email protected]/Native Method)
at java.net.SocketInputStream.socketRead([email protected]/Unknown Source)
at java.net.SocketInputStream.read([email protected]/Unknown Source)
at java.net.SocketInputStream.read([email protected]/Unknown Source)
at sun.security.ssl.SSLSocketInputRecord.read([email protected]/Unknown Source)
at sun.security.ssl.SSLSocketInputRecord.decode([email protected]/Unknown Source)
at sun.security.ssl.SSLSocketImpl.readRecord([email protected]/Unknown Source)
- locked <0x00000006c3343350> (a java.lang.Object)
at sun.security.ssl.SSLSocketImpl.readRecord([email protected]/Unknown Source)
at sun.security.ssl.SSLSocketImpl.performInitialHandshake([email protected]/Unknown Source)
- locked <0x00000006c3343448> (a java.lang.Object)
at sun.security.ssl.SSLSocketImpl.startHandshake([email protected]/Unknown Source)
at sun.security.ssl.SSLSocketImpl.startHandshake([email protected]/Unknown Source)
at sun.net.www.protocol.https.HttpsClient.afterConnect([email protected]/Unknown Source)
at sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.connect([email protected]/Unknown Source)
at sun.net.www.protocol.http.HttpURLConnection.getInputStream0([email protected]/Unknown Source)
- locked <0x00000006c33434c0> (a sun.net.www.protocol.https.DelegateHttpsURLConnection)
at sun.net.www.protocol.http.HttpURLConnection.getInputStream([email protected]/Unknown Source)
- locked <0x00000006c33434c0> (a sun.net.www.protocol.https.DelegateHttpsURLConnection)
at java.net.HttpURLConnection.getResponseCode([email protected]/Unknown Source)
at sun.net.www.protocol.https.HttpsURLConnectionImpl.getResponseCode([email protected]/Unknown Source)
at com.webcerebrium.binance.api.BinanceRequest.read(BinanceRequest.java:244)
at com.webcerebrium.binance.api.BinanceApi.account(BinanceApi.java:369)
at com.webcerebrium.binance.api.BinanceApi.balances(BinanceApi.java:378)
...

From my research this seems to be caused by a lack of timeout configuration on the http connection. I have added this setting to the where connections are created, with a very large timeout of 2 minutes, to avoid these hangs which go on indefinitely and can't be resolved without restarting the app.

I tested it here and the hangs are gone.

Thanks!

[wcrbrm]

thanks for the input @nosachamos
Do you occasionally have a suggestion where this timeout be specified as variable instead of being hardcoded as 120 seconds?

[nosachamos]

Humm.. It seems to me the best place would as a configuration, like the API key or secret. We could have another variable that the user can provide either from the application.properties file or using system properties that allows configuring the timeout.

In that case, I see that an instance of BinanceConfig is created within BinanceApi, but then it would need to be passed to each BinanceRequest during it's creation, which is not nice. How about we make BinanceConfig a singleton (loading the properties from the properties file only once), which will then enable it to be access by both the BinanceApi and each BinanceRequest instances? This way the BinanceRequest can read the timeout configuration from the BinanceConfig without the need of passing that instance around.

Thoughts?

If you agree I can make these changes.

Thanks!

Eduardo

[wcrbrm]

@nosachamos

• well, the main issue with singletons is typically loosing thread safety, so it may be not optimal too.
• BinanceConfig is for global variables, while timeout may be used with multiple values

Will try to post some solution, may be closer to the weekend, I feel that interface for using options would be something like this (preliminary)

BinanceApi api = new BinanceApi();
api.setRequestTimeout(true);
api.setUseServerTime(true);
// then, do requests

[nosachamos]

@wcrbrm sounds good, I will implement this approach and update the pull request.

Thank you!

Eduardo