NullPointerException in ConnectionManager.checkFallback: pendingConnect race between the null check and pendingConnect.host

[lishaan]

Version: io.ably:ably-android:1.7.2 (the code is unchanged in v1.8.0). Android, React Native app, ~21 unique users affected, fatal crash.

Stack (R8-minified; io.ably.lib.transport.d$d maps to ITransport$TransportParams, field b is host):

java.lang.NullPointerException: Attempt to read from field 'java.lang.String io.ably.lib.transport.d$d.b' on a null object reference
  at io.ably.lib.transport.ConnectionManager.checkFallback(ErrorInfo)
  at io.ably.lib.transport.ConnectionManager.onTransportUnavailable(ConnectionManager.java:1567)
  at io.ably.lib.transport.WebSocketTransport$WebSocketHandler.onError(WebSocketTransport.java:352)
  at io.ably.lib.transport.OkHttpWebSocketClient$WebSocketHandler.onFailure(OkHttpWebSocketClient.java:69)
  at okhttp3.internal.ws.RealWebSocket.failWebSocket

Analysis

In checkFallback (v1.7.2 ConnectionManager.java:1477), pendingConnect is null-checked at :1478 and then re-read at :1481/:1484, with a blocking network call in between: checkConnectivity() at :1479, which does HttpHelpers.getUrlString(..., INTERNET_CHECK_URL) at :1716.

onTransportUnavailable is synchronized, but neither writer of pendingConnect holds that monitor: connectImpl assigns it at :1609–1610 (its synchronized(this) block at :1623 only covers the transport swap) and ConnectedState.enact nulls it at :301. A concurrent connect/connected transition can therefore null the field mid-method, and the OkHttp onFailure thread throws. Nothing catches it, so the host process dies.

Repro conditions

Flaky mobile network, app backgrounded, reconnect loop with autoConnect = false and an authCallback. WebSocketTransport.WebSocketHandler.onError (:351) reports the failure as new ErrorInfo(message, 503, 80000), so the statusCode >= 500 gate at :1478 is satisfied and checkFallback proceeds into the blocking checkConnectivity().

A concrete interleaving that matches the stack: the transport connects and the CONNECTED protocol message arrives, so onConnected (synchronized, :1289) queues the connected-state action and releases the monitor. The socket then drops before the action runs. The OkHttp thread enters onTransportUnavailable (this.transport is still the same transport, currentState is still connecting), passes the null check at :1478 and blocks in checkConnectivity(). Meanwhile the ActionHandler thread runs the queued action via deferredAction.run() (:768), which is deliberately outside the monitor ("perform outstanding actions, without the ConnectionManager locked"), and ConnectedState.enact (:301) sets pendingConnect = null. The OkHttp thread resumes and dereferences pendingConnect.host at :1481. Not deterministically reproducible; we observe it continuously in production.

Suggested fix

Snapshot the field once (ConnectParams pending = pendingConnect; if (pending == null …)), and/or hold the ConnectionManager monitor for every read and write of pendingConnect.

Related

We also see main-thread ANRs bottoming out in ConnectionManager, consistent with checkConnectivity() performing a synchronous HTTP request while holding the monitor that onTransportUnavailable / checkSuspended also need.