IllegalStateException: transportShutdown() must be called before transportTerminated() when Android app goes to background

[pike-hachi]

What version of gRPC-Java are you using?

1.79.0

What is your environment?

• Operating System: Android (tested on Android 16)
• gRPC Transport: grpc-okhttp (via AndroidChannelBuilder)
• JDK: Android Runtime
• Dependencies:
  • grpc-android: 1.79.0
  • grpc-okhttp: 1.79.0
  • grpc-protobuf: 1.79.0
  • grpc-kotlin-stub: 1.5.0

What did you expect to see?

When the Android app is moved to the background, gRPC connections should gracefully handle the network state changes without crashing. The transport shutdown sequence should properly call transportShutdown() before transportTerminated().

What did you see instead?

The app consistently crashes approximately 5 seconds after being moved to the background:

FATAL EXCEPTION: OkHttpClientTransport
Process: com.notahotel.android.notahotel.dev, PID: 10823
java.lang.IllegalStateException: transportShutdown() must be called before transportTerminated().
    at com.google.common.base.Preconditions.checkState(Preconditions.java:513)
    at io.grpc.internal.InternalSubchannel$TransportListener.transportTerminated(InternalSubchannel.java:671)
    at io.grpc.okhttp.OkHttpClientTransport$ClientFrameHandler.run(OkHttpClientTransport.java:1347)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1154)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:652)
    at java.lang.Thread.run(Thread.java:1563)

Observations:

• The crash occurs on multiple OkHttpClientTransport threads simultaneously
• The timing is consistent: approximately 5 seconds after the app goes to background
• This issue does NOT occur with version 1.78.0 - only appears in 1.79.0

Steps to reproduce the bug

1. Set up an Android app using AndroidChannelBuilder with grpc-java 1.79.0:

val channel = AndroidChannelBuilder
    .forAddress(host, port)
    .useTransportSecurity()
    .context(applicationContext)
    .build()

2. Establish one or more active gRPC connections
3. Move the app to the background (press home button or switch to another app)
4. Wait approximately 5 seconds
5. Observe the crash in logcat

[shivaspeaks]

How frequent do you see this crash?

The problem seems to be a race and surfaced due to #12342. The crash is a result of a memory visibility issue between two threads:
Thread A (Shutdown Thread): When the Android app goes to the background, AndroidChannel triggers a channel shutdown(link). This thread calls transportShutdown() on the listener, which sets shutdownInitiated = true.
Thread B (OkHttp Thread): The OkHttpClientTransport frame handler thread is running its network loop . When the socket is closed by Thread A, Thread B exits its loop and calls transportTerminated() link.

Because shutdownInitiated is a plain boolean (not volatile and not always protected by the same lock at the point of the check), Thread B may not see the value true written by Thread A. It reads the stale value false and triggers the IllegalStateException.

The regression might have caused by plumbing the DisconnectError through the transport shutdown paths. Although the shutdownInitiated check was already in place, the changes in the shutdown sequence (specifically the interactions between KeepAliveManager and OkHttpClientTransport) exposed the lack of volatile or proper synchronization on that flag, leading to the crash when the Android OS restricts background network activity. Thats why my original question of how frequent do you see this crash to rule that its a race?

[pike-hachi]

Thanks for the quick response @shivaspeaks!

Regarding the frequency: This crash occurs consistently (100% reproducibility) when the app goes to background, not sporadically.
I tested this issue with Pixel8a(Android 16) and motorola razr 40 ultra (Android 15).

[ejona86]

@shivaspeaks, I think the race theory is a bit incomplete.

When the socket is closed by Thread A, Thread B exits its loop and calls transportTerminated() link.

All calls to listener.transportShutdown() are within a lock, and that lock is acquired in startGoAway() to make sure transportShutdown() is called before the transportTerminated().

When the Android app goes to the background, AndroidChannel triggers a channel shutdown

It triggers channel idleness, which triggers transport shutdown. I'm sure that's what you meant, but it is useful to keep the two straight.

---

I think the problem is an exception, which we don't see because of this second exception in the finally {}. Specifically, GrpcUtil.Http2Error.forCode() can return null, and new GoAwayDisconnectError(null) will throw an exception.

[ejona86]

FWIW, my thinking when I heard there may be a data race: the listener is not thread-safe (see ManagedClientTransport.Listener's javadoc), so the synchronization responsibility is on the transport. We normally handle that by only calling the listener on the network thread. In OkHttp the "network thread" is the lock, and we see the lock held for transportShutdown(). The lock isn't held when calling transportTerminated(), which may be a bug, or may be unnecessary because "no other callbacks may be called" (I'm not saying I've confirmed this, but it might allow the lock to be unnecessary). Even if the termination races with other callbacks (the lack of lock is a bug), I think we can prove that it doesn't race with transportShutdown().