search

BingAds / BingAds-Java-SDK

Other
42 stars 48 forks source link

Missing timeout on auth socket reading makes thread hang forever #109

Closed MarcelHB closed 5 years ago

MarcelHB commented 5 years ago

Hello,

For our Bing reporting, we started observing thread pool exhaustion lately over seemingly busy threads. A thread dump shows the following:

"pool-3-thread-1" #16 prio=5 os_prio=0 tid=0x00007f089547b800 nid=0x4a00 runnable [0x00007f087c6d0000]
   java.lang.Thread.State: RUNNABLE
    at java.net.SocketInputStream.socketRead0(Native Method)
    at java.net.SocketInputStream.socketRead(SocketInputStream.java:116)
    at java.net.SocketInputStream.read(SocketInputStream.java:171)
    at java.net.SocketInputStream.read(SocketInputStream.java:141)
    at sun.security.ssl.InputRecord.readFully(InputRecord.java:465)
    at sun.security.ssl.InputRecord.readV3Record(InputRecord.java:593)
    at sun.security.ssl.InputRecord.read(InputRecord.java:532)
    at sun.security.ssl.SSLSocketImpl.readRecord(SSLSocketImpl.java:983)
    - locked <0x00000000d3a4e3e0> (a java.lang.Object)
    at sun.security.ssl.SSLSocketImpl.readDataRecord(SSLSocketImpl.java:940)
    at sun.security.ssl.AppInputStream.read(AppInputStream.java:105)
    - locked <0x00000000d3a4e4a0> (a sun.security.ssl.AppInputStream)
    at org.apache.http.impl.io.AbstractSessionInputBuffer.fillBuffer(AbstractSessionInputBuffer.java:158)
    at org.apache.http.impl.io.SocketInputBuffer.fillBuffer(SocketInputBuffer.java:82)
    at org.apache.http.impl.io.AbstractSessionInputBuffer.readLine(AbstractSessionInputBuffer.java:271)
    at org.apache.http.impl.conn.DefaultHttpResponseParser.parseHead(DefaultHttpResponseParser.java:140)
    at org.apache.http.impl.conn.DefaultHttpResponseParser.parseHead(DefaultHttpResponseParser.java:57)
    at org.apache.http.impl.io.AbstractMessageParser.parse(AbstractMessageParser.java:259)
    at org.apache.http.impl.AbstractHttpClientConnection.receiveResponseHeader(AbstractHttpClientConnection.java:281)
    at org.apache.http.impl.conn.DefaultClientConnection.receiveResponseHeader(DefaultClientConnection.java:259)
    at org.apache.http.impl.conn.ManagedClientConnectionImpl.receiveResponseHeader(ManagedClientConnectionImpl.java:209)
    at org.apache.http.protocol.HttpRequestExecutor.doReceiveResponse(HttpRequestExecutor.java:273)
    at org.apache.http.protocol.HttpRequestExecutor.execute(HttpRequestExecutor.java:125)
    at org.apache.http.impl.client.DefaultRequestDirector.tryExecute(DefaultRequestDirector.java:686)
    at org.apache.http.impl.client.DefaultRequestDirector.execute(DefaultRequestDirector.java:488)
    at org.apache.http.impl.client.AbstractHttpClient.doExecute(AbstractHttpClient.java:884)
    at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:82)
    at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:107)
    at com.microsoft.bingads.internal.HttpClientWebServiceCaller.post(HttpClientWebServiceCaller.java:31)
    at com.microsoft.bingads.internal.UriOAuthService.getAccessTokens(UriOAuthService.java:90)
    at com.microsoft.bingads.internal.OAuthWithAuthorizationCode.requestAccessAndRefreshTokens(OAuthWithAuthorizationCode.java:144)
    at ...createAuthorizationData(BingUser.java:97)

Our logs, and also this issue give the impression that it is mostly an issue for the auth request step.

Allowing the user to (optionally) set timeouts for this kind of network operations would help us looking for SocketTimeoutExceptions or similar, thus allowing the recycling of the thread for a next attempt. We haven't discovered such an option in the Bing Ads SDK.

In some rare cases, we received a java.net.SocketException: Connection reset but this is not a behavior to rely on, obviously.

Thank you! Marcel

eric-urban commented 5 years ago

@MarcelHB have you tried version 12.13.4? We recently added a timeout. Does this help?

MarcelHB commented 5 years ago

Thanks! My bad, I had not checked the newest release notes. That looks good, we'll monitor that but consider the issue closed. :slightly_smiling_face: