Skip to content

Conversation

@gedeagas
Copy link
Contributor

@gedeagas gedeagas commented Jul 7, 2021

Summary

During our routine crash report check we are occasionally seeing reports of exceptions like this in the wild from our crash stack:

java.lang.NullPointerException: bio == null
       at com.android.org.conscrypt.NativeCrypto.SSL_pending_written_bytes_in_BIO(NativeCrypto.java)
       at com.android.org.conscrypt.NativeSsl$BioWrapper.getPendingWrittenBytes(NativeSsl.java:660)
       at com.android.org.conscrypt.ConscryptEngine.pendingOutboundEncryptedBytes(ConscryptEngine.java:566)
       at com.android.org.conscrypt.ConscryptEngineSocket.drainOutgoingQueue(ConscryptEngineSocket.java:584)
       at com.android.org.conscrypt.ConscryptEngineSocket.close(ConscryptEngineSocket.java:480)
       at okhttp3.internal.Util.closeQuietly(Util.kt:501)
       at okhttp3.internal.connection.ExchangeFinder.findConnection(ExchangeFile:245)
       at okhttp3.internal.connection.ExchangeFinder.findHealthyConnection(ExchangeFile:106)
       at okhttp3.internal.connection.ExchangeFinder.find(ExchangeFile:74)
       at okhttp3.internal.connection.RealCall.initExchange$okhttp(ExchangeFile:255)
       at okhttp3.internal.connection.ConnectInterceptor.intercept(ExchangeFile:32)
       ...

Screen Shot 2021-07-07 at 1 38 23 PM

This appears to only be happening on devices running Android 10 and 11. This happens because there is concurrency issue in Conscrypt where two threads race to close an SSLEngine-based SSLSocket and access to the underlying BIO is unsynchronized.

The OkHttp team already released a fix for this issue on version 4.9.1 this PR aims to update our OkHttp package to version 4.9.1.

Related discussion:
https://issuetracker.google.com/issues/177450597
https://publicobject.com/2021/01/30/bio-null/

cc @dulmandakh @fkgozali

Changelog

[Android] [Changed] - Bumping OkHttp from 4.9.0 to 4.9.1.

Test Plan

Manual & Automated from CI

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 7, 2021
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: d85d72d

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 9,120,511 -22
android hermes armeabi-v7a 8,646,412 -20
android hermes x86 9,559,714 -30
android hermes x86_64 9,525,872 -22
android jsc arm64-v8a 10,762,957 -21
android jsc armeabi-v7a 9,680,118 -20
android jsc x86 10,797,556 -29
android jsc x86_64 11,405,230 -9

Base commit: d85d72d

Copy link
Contributor

@dulmandakh dulmandakh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ShikaSD please merge

@facebook-github-bot
Copy link
Contributor

@fkgozali has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@ShikaSD merged this pull request in 6caec9d.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Jul 8, 2021
grabbou pushed a commit that referenced this pull request Jul 16, 2021
…lPointerException: bio == null crash (#31822)

Summary:
Douring our routine crash report check we are occasionally seeing reports of exceptions like this in the wild from our crash stack:

```
java.lang.NullPointerException: bio == null
       at com.android.org.conscrypt.NativeCrypto.SSL_pending_written_bytes_in_BIO(NativeCrypto.java)
       at com.android.org.conscrypt.NativeSsl$BioWrapper.getPendingWrittenBytes(NativeSsl.java:660)
       at com.android.org.conscrypt.ConscryptEngine.pendingOutboundEncryptedBytes(ConscryptEngine.java:566)
       at com.android.org.conscrypt.ConscryptEngineSocket.drainOutgoingQueue(ConscryptEngineSocket.java:584)
       at com.android.org.conscrypt.ConscryptEngineSocket.close(ConscryptEngineSocket.java:480)
       at okhttp3.internal.Util.closeQuietly(Util.kt:501)
       at okhttp3.internal.connection.ExchangeFinder.findConnection(ExchangeFile:245)
       at okhttp3.internal.connection.ExchangeFinder.findHealthyConnection(ExchangeFile:106)
       at okhttp3.internal.connection.ExchangeFinder.find(ExchangeFile:74)
       at okhttp3.internal.connection.RealCall.initExchange$okhttp(ExchangeFile:255)
       at okhttp3.internal.connection.ConnectInterceptor.intercept(ExchangeFile:32)
       ...
  ```

![Screen Shot 2021-07-07 at 1 38 23 PM](https://user-images.githubusercontent.com/8868908/124711795-b5fee980-df28-11eb-98c4-9668661340b6.png)

This appears to only be happening on devices running Android 10 and 11. This happens because there is concurrency issue in Conscrypt where two threads race to close an SSLEngine-based SSLSocket and access to the underlying BIO is unsynchronized.

 **The OkHttp team already released a fix for this issue on version 4.9.1** this PR aims to update our OkHttp package to version 4.9.1.

 Related discussion:
 [https://issuetracker.google.com/issues/177450597](https://issuetracker.google.com/issues/177450597)
 [https://publicobject.com/2021/01/30/bio-null/](https://publicobject.com/2021/01/30/bio-null/)

cc dulmandakh fkgozali

## Changelog
[Android] [Changed] - Bumping OkHttp from 4.9.0 to 4.9.1.

Pull Request resolved: #31822

Test Plan: Manual & Automated from CI

Reviewed By: fkgozali

Differential Revision: D29590198

Pulled By: ShikaSD

fbshipit-source-id: 4228bfd3472114253e13acb436dc1dd9287a148d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Needs: React Native Team Attention

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants