Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unhandled java.lang.NullPointerException: bio == null in io.grpc.okhttp.internal.framed.Http2$Reader.close #10800

Closed
ezrafernandez-toast opened this issue Jan 4, 2024 · 5 comments · Fixed by #10811
Assignees
Milestone

Comments

@ezrafernandez-toast
Copy link

What version of gRPC-Java are you using?

1.47.0

What is your environment?

Android 8, 9, 10, 12.

What did you expect to see?

When the underlying socket fails to close, possibly due to a race condition where it's already been closed, handle the thrown NPE.

What did you see instead?

An NPE is thrown and causes a crash.
https://issuetracker.google.com/issues/177450597
https://github.com/square/okhttp/pull/6533/files#diff-f88f9ac1ee194722f304572130663d2ba659e6e571fbc6dbfc6e639afe4003a4R502

Steps to reproduce the bug

Stack from a production report haven't been able to reproduce locally.

java.lang.NullPointerException: bio == null
    at org.conscrypt.NativeCrypto.SSL_pending_written_bytes_in_BIO(NativeCrypto.java)
    at org.conscrypt.NativeSsl$BioWrapper.getPendingWrittenBytes(NativeSsl.java:659)
    at org.conscrypt.ConscryptEngine.pendingOutboundEncryptedBytes(ConscryptEngine.java:565)
    at org.conscrypt.ConscryptEngineSocket.drainOutgoingQueue(ConscryptEngineSocket.java:568)
    at org.conscrypt.ConscryptEngineSocket.close(ConscryptEngineSocket.java:464)
    at org.conscrypt.ConscryptEngineSocket$SSLInputStream.close(ConscryptEngineSocket.java:732)
    at okio.InputStreamSource.close(JvmOkio.kt:111)
    at okio.AsyncTimeout$source$1.close(AsyncTimeout.kt:132)
    at okio.RealBufferedSource.close(RealBufferedSource.kt:480)
    at io.grpc.okhttp.internal.framed.Http2$Reader.close(Http2.java:370)
    at io.grpc.okhttp.OkHttpClientTransport$ClientFrameHandler.run(OkHttpClientTransport.java:1104)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
    at java.lang.Thread.run(Thread.java:764)
@sergiitk
Copy link
Member

sergiitk commented Jan 9, 2024

@temawi - preliminary investigation shows it's legit, please take a look at this code path:

try {
frameReader.close();
} catch (IOException ex) {
log.log(Level.INFO, "Exception closing frame reader", ex);
}

Would it make sense to do another catch block and check the description to be bio == null similar to what okhttp3 does in the PR diff linked by the OP?

@temawi
Copy link
Contributor

temawi commented Jan 10, 2024

Probably no harm in doing that, but there's just one thing I'm wondering about. The Conscrypt bug mentions that this problem can occur on Android 9 or 10, but this issue mentions it also happening on Android 12.

@ezrafernandez-toast Can you confirm that you really are seeing this on Android 12 devices as well?

@ezrafernandez-toast
Copy link
Author

The majority of occurrences are on lower versions, with < 10% of reports on Android 12. I suppose it's possible the Conscrypt module has not updated on those devices?

@temawi
Copy link
Contributor

temawi commented Jan 10, 2024

I could buy that. We can get this change in to v1.61.0 that is releasing in the next few days, but there would not be patch release for v1.47.0 that you are using.

Is this actually causing you problems besides the occasional error message? The connection is already closed so hopefully you are not seeing any actual failed RPCs.

@temawi temawi added this to the 1.61 milestone Jan 10, 2024
@ezrafernandez-toast
Copy link
Author

I can nudge the team to update to 1.61. Our issue tracker reports these as fatal crashes.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants