Thoughts on virtual thread pinning caused by classic synchronization
Thilo-Alexander Ginkel
thilo at ginkel.com
Tue Aug 10 11:14:02 UTC 2021
Hello Alan,
thanks for your reply!
On Sun, Aug 8, 2021 at 3:35 PM Alan Bateman <Alan.Bateman at oracle.com> wrote:
> The java.io APIs are problematic as they have used synchronization since
> JDK 1.0/1.1. In most cases the synchronization is not specified and not
> enforced consistently. In other cases there is a lock object exposed as
> a protected field for sub-classes. In general, most of these APIs should
> not need synchronization but there are a few exceptions (PrintStream
> intended to be written too by concurrent threads, async close, cases
> where the underlying transport is packet based, and a few
> security-sensitive scenarios where corrupting the stream is a concern).
> So we have to proceed with caution. The approach that we've taken is to
> use j.u.c. locks when not sub-classed or when sub-classes by trusted/JDK
> classes. There is still a risk that there is code in the wild that makes
> assumptions on how synchronizes on the stream but I don't think we can
> do anything about that. At some point, and probably over several
> releases, we will need to address the technical debt in this area and
> gradually remove the often-broken and unspecified synchronization.
>
> You mentioned PushbackInputStream and you shouldn't have an issue there,
> unless you are wrapping something that is using synchronization.
Actually, for PushbackInputStream we see thread pinning when the wrapped
InputStream from Apache HttpComponents 5 is being closed and wants to
return the underlying connection to the connection pool (which may block):
"ForkJoinPool-1-worker-2" #87 [91] daemon prio=5 os_prio=0 cpu=164.20ms
elapsed=109.16s tid=0x00007fa8540119b0 nid=0x5b waiting on condition
[0x00007fa832cea000]
java.lang.Thread.State: WAITING (parking)
at jdk.internal.misc.Unsafe.park(java.base at 17-loom/Native Method)
- parking to wait for <0x000010001e990c68> (a
java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject)
at java.util.concurrent.locks.LockSupport.park(java.base at 17-loom
/LockSupport.java:371)
at
java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionNode.block(java.base at 17-loom
/AbstractQueuedSynchronizer.java:506)
at
java.util.concurrent.ForkJoinPool.compensatedBlock(java.base at 17-loom
/ForkJoinPool.java:3456)
at java.util.concurrent.ForkJoinPool.managedBlock(java.base at 17-loom
/ForkJoinPool.java:3439)
at
java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.await(java.base at 17-loom
/AbstractQueuedSynchronizer.java:1623)
at java.lang.VirtualThread.parkOnCarrierThread(java.base at 17-loom
/VirtualThread.java:469)
at java.lang.VirtualThread.yieldContinuation(java.base at 17-loom
/VirtualThread.java:379)
at java.lang.VirtualThread.park(java.base at 17-loom
/VirtualThread.java:534)
at java.lang.System$2.parkVirtualThread(java.base at 17-loom
/System.java:2373)
at jdk.internal.misc.VirtualThreads.park(java.base at 17-loom
/VirtualThreads.java:60)
at java.util.concurrent.locks.LockSupport.park(java.base at 17-loom
/LockSupport.java:219)
at
java.util.concurrent.locks.AbstractQueuedSynchronizer.acquire(java.base at 17-loom
/AbstractQueuedSynchronizer.java:715)
at
java.util.concurrent.locks.AbstractQueuedSynchronizer.acquire(java.base at 17-loom
/AbstractQueuedSynchronizer.java:938)
at
java.util.concurrent.locks.ReentrantLock$Sync.lock(java.base at 17-loom
/ReentrantLock.java:153)
at java.util.concurrent.locks.ReentrantLock.lock(java.base at 17-loom
/ReentrantLock.java:322)
at
org.apache.hc.core5.pool.StrictConnPool.release(StrictConnPool.java:238)
at
org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager.release(PoolingHttpClientConnectionManager.java:383)
at
org.apache.hc.client5.http.impl.classic.InternalExecRuntime.releaseEndpoint(InternalExecRuntime.java:261)
at
org.apache.hc.client5.http.impl.classic.ResponseEntityProxy.releaseConnection(ResponseEntityProxy.java:80)
at
org.apache.hc.client5.http.impl.classic.ResponseEntityProxy.streamClosed(ResponseEntityProxy.java:137)
at
org.apache.hc.core5.http.io.EofSensorInputStream.checkClose(EofSensorInputStream.java:228)
at
org.apache.hc.core5.http.io.EofSensorInputStream.close(EofSensorInputStream.java:172)
at
com.example.ApacheHttpClientDownloadHandler$ResponseClosingInputStream.close(ApacheHttpClientDownloadHandler.java:91)
at org.apache.commons.io.IOUtils.close(IOUtils.java:403)
at
org.apache.commons.io.input.ProxyInputStream.close(ProxyInputStream.java:148)
at java.io.PushbackInputStream.close(java.base at 17-loom
/PushbackInputStream.java:377)
- locked <0x0000105fffb0e3c0> (a java.io.PushbackInputStream)
at
org.apache.commons.compress.archivers.zip.ZipArchiveInputStream.close(ZipArchiveInputStream.java:673)
[...]
This is using a patched HttpComponents version that replaces synchronized
with ReetrantLock (hoping to eliminate all synchronized blocks on code
paths discovered during our tests) - the original version
of org.apache.hc.core5.pool.StrictConnPool.release is synchronized.
I guess the PushbackInputStream is one of those cases that can't be easily
migrated due to the synchronized object ("this") being accessible by
subclasses.
You also mentioned FileChannel - if you have concurrent threads accessing a
> file channel then they will probably be using the positional read/write
> methods that are designed to support concurrent access to different
> parts of a file.
>
sun.nio.ch.FileChannelImpl#write(java.nio.ByteBuffer) synchronizes on a
positionLock, so I guess your assumption is correct:
"ForkJoinPool-1-worker-42" #39299 [137] daemon prio=5 os_prio=0
cpu=2867.67ms elapsed=25.18s tid=0x00007f9f8909fb30 nid=0x89 runnable
[0x00007f9ee64eb000]
java.lang.Thread.State: RUNNABLE
at jdk.internal.misc.Unsafe.unpark(java.base at 17-loom/Native Method)
- parking to wait for <0x0000105ffbe00180> (a
java.util.concurrent.ForkJoinPool)
at java.util.concurrent.locks.LockSupport.unpark(java.base at 17-loom
/LockSupport.java:181)
at java.util.concurrent.ForkJoinPool.tryCompensate(java.base at 17-loom
/ForkJoinPool.java:1791)
at
java.util.concurrent.ForkJoinPool.compensatedBlock(java.base at 17-loom
/ForkJoinPool.java:3453)
at
java.lang.invoke.LambdaForm$DMH/0x0000000801001400.invokeSpecial(java.base at 17-loom
/LambdaForm$DMH)
at
java.lang.invoke.LambdaForm$MH/0x0000000801934000.invoke_MT(java.base at 17-loom
/LambdaForm$MH)
at jdk.internal.misc.Blocker.managedBlock(java.base at 17-loom
/Blocker.java:159)
at sun.nio.ch.FileChannelImpl.write(java.base at 17-loom
/FileChannelImpl.java:296)
- locked <0x0000100148580948> (a java.lang.Object)
at java.nio.channels.Channels.writeFullyImpl(java.base at 17-loom
/Channels.java:74)
at java.nio.channels.Channels.writeFully(java.base at 17-loom
/Channels.java:96)
at java.nio.channels.Channels$1.write(java.base at 17-loom
/Channels.java:171)
- locked <0x00001001485809d0> (a java.nio.channels.Channels$1)
at org.apache.commons.io.IOUtils.copyLarge(IOUtils.java:1310)
at org.apache.commons.io.IOUtils.copy(IOUtils.java:978)
at org.apache.commons.io.IOUtils.copyLarge(IOUtils.java:1282)
at org.apache.commons.io.IOUtils.copy(IOUtils.java:953)
[...]
For Channels, it's an anonymous inner class.
Both could be converted to ReentrantLocks, as they are not accessible via
inheritance and thus not part of a contract, right?
What would be your recommendation? Wait until synchronized support is in
place or try to migrate problematic libraries to ReentrantLock? I
understand that the latter may not be feasible for some JDK classes due to
possible contract violations...
Thanks,
Thilo
More information about the loom-dev
mailing list