Virtual thread pinning using Conscrypt with Socket APIs
- Dominant language
- Java
- Stars
- 1.4k
- Forks
- 326
- Avg merge
- 16h 22m
- Merged PRs (30d)
- 17
Description
## Background
Conscrypt offers excellent performance, which is helpful now more than ever given the [TLS performance regressions on x86_64 in JDK-18+](https://mail.openjdk.org/pipermail/security-dev/2022-November/032757.html). Unfortunately the Conscrypt Socket stream implementations synchronize over Socket I/O pinning virtual threads to carriers as described in [jep-444](https://openjdk.org/jeps/444). This prevents Conscrypt users from taking advantage of virtual threads (in fact introduces the deadlocks as all carriers become pinned).
Snippet from a unit test where enabling virtual threads resulted in a deadlock:
```
java.base/sun.nio.ch.NioSocketImpl.read(NioSocketImpl.java:346)
java.base/sun.nio.ch.NioSocketImpl$1.read(NioSocketImpl.java:796)
java.base/java.net.Socket$SocketInputStream.read(Socket.java:1099)
org.conscrypt.ConscryptEngineSocket$SSLInputStream.readFromSocket(ConscryptEngineSocket.java:920)
org.conscrypt.ConscryptEngineSocket$SSLInputStream.processDataFromSocket(ConscryptEngineSocket.java:884)
org.conscrypt.ConscryptEngineSocket$SSLInputStream.readUntilDataAvailable(ConscryptEngineSocket.java:799)
org.conscrypt.ConscryptEngineSocket$SSLInputStream.read(ConscryptEngineSocket.java:772) <== monitors:1
org.apache.hc.core5.http.impl.io.SessionInputBufferImpl.fillBuffer(SessionInputBufferImpl.java:149)
org.apache.hc.core5.http.impl.io.SessionInputBufferImpl.readLine(SessionInputBufferImpl.java:280)
org.apache.hc.core5.http.impl.io.ChunkedInputStream.getChunkSize(ChunkedInputStream.java:261)
```
## Proposed Solution
We should be able to retain existing behavior by replacing Object monitor locks with ReentrantLocks, which do not pin virtual threads. Such a change would need to be performance tested, and may impact observability (e.g. deadlock detection, thread dump metadata).
Does this seem reasonable? Would you accept such a contribution if I were to implement it?
Thanks!
Contributor guide
Assessment
This issue has not been assessed yet.