Merge pull request #1119 from rjbaucells/virtual-threads

michaelklishin · web-flow · commit 06346eda7711 · 2023-09-24T21:06:42.000-04:00
Prevent pinned CarrierThreads on JDK-21 while using Virtual Threads
diff --git a/src/main/java/com/rabbitmq/client/impl/AMQChannel.java b/src/main/java/com/rabbitmq/client/impl/AMQChannel.java
@@ -31,6 +31,8 @@
 import java.io.IOException;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.TimeoutException;
+import java.util.concurrent.locks.Condition;
+import java.util.concurrent.locks.ReentrantLock;
 import java.util.function.Supplier;
 
 /**
@@ -54,7 +56,8 @@ public abstract class AMQChannel extends ShutdownNotifierComponent {
      * so that clients can themselves use the channel to synchronize
      * on.
      */
-    protected final Object _channelMutex = new Object();
+    protected final ReentrantLock _channelLock = new ReentrantLock();
+    protected final Condition _channelLockCondition = _channelLock.newCondition();
 
     /** The connection this channel is associated with. */
     private final AMQConnection _connection;
@@ -191,14 +194,17 @@ public void handleCompleteInboundCommand(AMQCommand command) throws IOException
             // so it must be a response to an earlier RPC.
 
             if (_checkRpcResponseType) {
-                synchronized (_channelMutex) {
+                _channelLock.lock();
+                try {
                     // check if this reply command is intended for the current waiting request before calling nextOutstandingRpc()
                     if (_activeRpc != null && !_activeRpc.canHandleReply(command)) {
                         // this reply command is not intended for the current waiting request
                         // most likely a previous request timed out and this command is the reply for that.
                         // Throw this reply command away so we don't stop the current request from waiting for its reply
                         return;
                     }
+                } finally {
+                    _channelLock.unlock();
                 }
             }
             final RpcWrapper nextOutstandingRpc = nextOutstandingRpc();
@@ -220,11 +226,12 @@ public void enqueueAsyncRpc(Method method, CompletableFuture<Command> future) {
     }
 
     private void doEnqueueRpc(Supplier<RpcWrapper> rpcWrapperSupplier) {
-        synchronized (_channelMutex) {
+        _channelLock.lock();
+        try {
             boolean waitClearedInterruptStatus = false;
             while (_activeRpc != null) {
                 try {
-                    _channelMutex.wait();
+                    _channelLockCondition.await();
                 } catch (InterruptedException e) { //NOSONAR
                     waitClearedInterruptStatus = true;
                     // No Sonar: we re-interrupt the thread later
@@ -234,23 +241,31 @@ private void doEnqueueRpc(Supplier<RpcWrapper> rpcWrapperSupplier) {
                 Thread.currentThread().interrupt();
             }
             _activeRpc = rpcWrapperSupplier.get();
+        } finally {
+            _channelLock.unlock();
         }
     }
 
     public boolean isOutstandingRpc()
     {
-        synchronized (_channelMutex) {
+        _channelLock.lock();
+        try {
             return (_activeRpc != null);
+        } finally {
+            _channelLock.unlock();
         }
     }
 
     public RpcWrapper nextOutstandingRpc()
     {
-        synchronized (_channelMutex) {
+        _channelLock.lock();
+        try {
             RpcWrapper result = _activeRpc;
             _activeRpc = null;
-            _channelMutex.notifyAll();
+            _channelLockCondition.signalAll();
             return result;
+        } finally {
+            _channelLock.unlock();
         }
     }
 
@@ -344,36 +359,48 @@ private AMQCommand privateRpc(Method m, int timeout)
     public void rpc(Method m, RpcContinuation k)
         throws IOException
     {
-        synchronized (_channelMutex) {
+        _channelLock.lock();
+        try {
             ensureIsOpen();
             quiescingRpc(m, k);
+        } finally {
+            _channelLock.unlock();
         }
     }
 
     public void quiescingRpc(Method m, RpcContinuation k)
         throws IOException
     {
-        synchronized (_channelMutex) {
+        _channelLock.lock();
+        try {
             enqueueRpc(k);
             quiescingTransmit(m);
+        } finally {
+            _channelLock.unlock();
         }
     }
 
     public void asyncRpc(Method m, CompletableFuture<Command> future)
         throws IOException
     {
-        synchronized (_channelMutex) {
+        _channelLock.lock();
+        try {
             ensureIsOpen();
             quiescingAsyncRpc(m, future);
+        } finally {
+            _channelLock.unlock();
         }
     }
 
     public void quiescingAsyncRpc(Method m, CompletableFuture<Command> future)
         throws IOException
     {
-        synchronized (_channelMutex) {
+        _channelLock.lock();
+        try {
             enqueueAsyncRpc(m, future);
             quiescingTransmit(m);
+        } finally {
+            _channelLock.unlock();
         }
     }
 
@@ -402,13 +429,16 @@ public void processShutdownSignal(ShutdownSignalException signal,
                                       boolean ignoreClosed,
                                       boolean notifyRpc) {
         try {
-            synchronized (_channelMutex) {
+            _channelLock.lock();
+            try {
                 if (!setShutdownCauseIfOpen(signal)) {
                     if (!ignoreClosed)
                         throw new AlreadyClosedException(getCloseReason());
                 }
 
-                _channelMutex.notifyAll();
+                _channelLockCondition.signalAll();
+            } finally {
+                _channelLock.unlock();
             }
         } finally {
             if (notifyRpc)
@@ -424,30 +454,40 @@ public void notifyOutstandingRpc(ShutdownSignalException signal) {
     }
 
     public void transmit(Method m) throws IOException {
-        synchronized (_channelMutex) {
+        _channelLock.lock();
+        try {
             transmit(new AMQCommand(m));
+        } finally {
+            _channelLock.unlock();
         }
     }
 
     public void transmit(AMQCommand c) throws IOException {
-        synchronized (_channelMutex) {
+        _channelLock.lock();
+        try {
             ensureIsOpen();
             quiescingTransmit(c);
+        } finally {
+            _channelLock.unlock();
         }
     }
 
     public void quiescingTransmit(Method m) throws IOException {
-        synchronized (_channelMutex) {
+        _channelLock.lock();
+        try {
             quiescingTransmit(new AMQCommand(m));
+        } finally {
+            _channelLock.unlock();
         }
     }
 
     public void quiescingTransmit(AMQCommand c) throws IOException {
-        synchronized (_channelMutex) {
+        _channelLock.lock();
+        try {
             if (c.getMethod().hasContent()) {
                 while (_blockContent) {
                     try {
-                        _channelMutex.wait();
+                        _channelLockCondition.await();
                     } catch (InterruptedException ignored) {
                         Thread.currentThread().interrupt();
                     }
@@ -460,6 +500,8 @@ public void quiescingTransmit(AMQCommand c) throws IOException {
             }
             this._trafficListener.write(c);
             c.transmit(this);
+        } finally {
+            _channelLock.unlock();
         }
     }
 
diff --git a/src/main/java/com/rabbitmq/client/impl/AMQCommand.java b/src/main/java/com/rabbitmq/client/impl/AMQCommand.java
@@ -18,6 +18,7 @@
 import java.io.ByteArrayOutputStream;
 import java.io.DataOutputStream;
 import java.io.IOException;
+import java.util.concurrent.locks.ReentrantLock;
 
 import com.rabbitmq.client.AMQP;
 import com.rabbitmq.client.Command;
@@ -43,6 +44,7 @@ public class AMQCommand implements Command {
 
     /** The assembler for this command - synchronised on - contains all the state */
     private final CommandAssembler assembler;
+    private final ReentrantLock assemblerLock = new ReentrantLock();
 
     AMQCommand(int maxBodyLength) {
         this(null, null, null, maxBodyLength);
@@ -115,7 +117,8 @@ public void transmit(AMQChannel channel) throws IOException {
         int channelNumber = channel.getChannelNumber();
         AMQConnection connection = channel.getConnection();
 
-        synchronized (assembler) {
+        assemblerLock.lock();
+        try {
             Method m = this.assembler.getMethod();
             if (m.hasContent()) {
                 byte[] body = this.assembler.getContentBody();
@@ -145,6 +148,8 @@ public void transmit(AMQChannel channel) throws IOException {
             } else {
                 connection.writeFrame(m.toFrame(channelNumber));
             }
+        } finally {
+            assemblerLock.unlock();
         }
 
         connection.flush();
@@ -155,7 +160,8 @@ public void transmit(AMQChannel channel) throws IOException {
     }
 
     public String toString(boolean suppressBody){
-        synchronized (assembler) {
+        assemblerLock.lock();
+        try {
             return new StringBuilder()
                 .append('{')
                 .append(this.assembler.getMethod())
@@ -165,6 +171,8 @@ public String toString(boolean suppressBody){
                 .append(contentBodyStringBuilder(
                         this.assembler.getContentBody(), suppressBody))
                 .append('}').toString();
+        } finally {
+            assemblerLock.unlock();
         }
     }
 
diff --git a/src/main/java/com/rabbitmq/client/impl/ChannelN.java b/src/main/java/com/rabbitmq/client/impl/ChannelN.java
@@ -361,10 +361,13 @@ private void releaseChannel() {
                 return true;
             } else if (method instanceof Channel.Flow) {
                 Channel.Flow channelFlow = (Channel.Flow) method;
-                synchronized (_channelMutex) {
+                _channelLock.lock();
+                try {
                     _blockContent = !channelFlow.getActive();
                     transmit(new Channel.FlowOk(!_blockContent));
-                    _channelMutex.notifyAll();
+                    _channelLockCondition.signalAll();
+                } finally {
+                    _channelLock.unlock();
                 }
                 return true;
             } else if (method instanceof Basic.Ack) {
@@ -524,7 +527,8 @@ private void asyncShutdown(Command command) throws IOException {
                                                                      false,
                                                                      command.getMethod(),
                                                                      this);
-        synchronized (_channelMutex) {
+        _channelLock.lock();
+        try {
             try {
                 processShutdownSignal(signal, true, false);
                 quiescingTransmit(new Channel.CloseOk());
@@ -533,6 +537,9 @@ private void asyncShutdown(Command command) throws IOException {
                 notifyOutstandingRpc(signal);
             }
         }
+        finally {
+            _channelLock.unlock();
+        }
         notifyListeners();
     }
 
@@ -612,9 +619,12 @@ public AMQCommand transformReply(AMQCommand command) {
         try {
             // Synchronize the block below to avoid race conditions in case
             // connection wants to send Connection-CloseOK
-            synchronized (_channelMutex) {
+            _channelLock.lock();
+            try {
                 startProcessShutdownSignal(signal, !initiatedByApplication, true);
                 quiescingRpc(reason, k);
+            } finally {
+                _channelLock.unlock();
             }
 
             // Now that we're in quiescing state, channel.close was sent and
@@ -1602,16 +1612,22 @@ public CompletableFuture<Command> asyncCompletableRpc(Method method) throws IOEx
 
     @Override
     public void enqueueRpc(RpcContinuation k) {
-        synchronized (_channelMutex) {
+        _channelLock.lock();
+        try {
             super.enqueueRpc(k);
             dispatcher.setUnlimited(true);
+        } finally {
+            _channelLock.unlock();
         }
     }
 
     @Override
     protected void markRpcFinished() {
-        synchronized (_channelMutex) {
+        _channelLock.lock();
+        try {
             dispatcher.setUnlimited(false);
+        } finally {
+            _channelLock.unlock();
         }
     }
 
diff --git a/src/main/java/com/rabbitmq/client/impl/SocketFrameHandler.java b/src/main/java/com/rabbitmq/client/impl/SocketFrameHandler.java

Original file line number	Diff line number	Diff line change
`@@ -31,6 +31,8 @@`
`31`	`31`	`import java.io.IOException;`
`32`	`32`	`import java.util.concurrent.CompletableFuture;`
`33`	`33`	`import java.util.concurrent.TimeoutException;`
	`34`	`+import java.util.concurrent.locks.Condition;`
	`35`	`+import java.util.concurrent.locks.ReentrantLock;`
`34`	`36`	`import java.util.function.Supplier;`
`35`	`37`
`36`	`38`	`/**`
`@@ -54,7 +56,8 @@ public abstract class AMQChannel extends ShutdownNotifierComponent {`
`54`	`56`	`* so that clients can themselves use the channel to synchronize`
`55`	`57`	`* on.`
`56`	`58`	`*/`
`57`		`- protected final Object _channelMutex = new Object();`
	`59`	`+ protected final ReentrantLock _channelLock = new ReentrantLock();`
	`60`	`+ protected final Condition _channelLockCondition = _channelLock.newCondition();`
`58`	`61`
`59`	`62`	`/** The connection this channel is associated with. */`
`60`	`63`	`private final AMQConnection _connection;`
`@@ -191,14 +194,17 @@ public void handleCompleteInboundCommand(AMQCommand command) throws IOException`
`191`	`194`	`// so it must be a response to an earlier RPC.`
`192`	`195`
`193`	`196`	`if (_checkRpcResponseType) {`
`194`		`- synchronized (_channelMutex) {`
	`197`	`+ _channelLock.lock();`
	`198`	`+ try {`
`195`	`199`	`// check if this reply command is intended for the current waiting request before calling nextOutstandingRpc()`
`196`	`200`	`if (_activeRpc != null && !_activeRpc.canHandleReply(command)) {`
`197`	`201`	`// this reply command is not intended for the current waiting request`
`198`	`202`	`// most likely a previous request timed out and this command is the reply for that.`
`199`	`203`	`// Throw this reply command away so we don't stop the current request from waiting for its reply`
`200`	`204`	`return;`
`201`	`205`	`}`
	`206`	`+ } finally {`
	`207`	`+ _channelLock.unlock();`
`202`	`208`	`}`
`203`	`209`	`}`
`204`	`210`	`final RpcWrapper nextOutstandingRpc = nextOutstandingRpc();`
`@@ -220,11 +226,12 @@ public void enqueueAsyncRpc(Method method, CompletableFuture<Command> future) {`
`220`	`226`	`}`
`221`	`227`
`222`	`228`	`private void doEnqueueRpc(Supplier<RpcWrapper> rpcWrapperSupplier) {`
`223`		`- synchronized (_channelMutex) {`
	`229`	`+ _channelLock.lock();`
	`230`	`+ try {`
`224`	`231`	`boolean waitClearedInterruptStatus = false;`
`225`	`232`	`while (_activeRpc != null) {`
`226`	`233`	`try {`
`227`		`- _channelMutex.wait();`
	`234`	`+ _channelLockCondition.await();`
`228`	`235`	`} catch (InterruptedException e) { //NOSONAR`
`229`	`236`	`waitClearedInterruptStatus = true;`
`230`	`237`	`// No Sonar: we re-interrupt the thread later`
`@@ -234,23 +241,31 @@ private void doEnqueueRpc(Supplier<RpcWrapper> rpcWrapperSupplier) {`
`234`	`241`	`Thread.currentThread().interrupt();`
`235`	`242`	`}`
`236`	`243`	`_activeRpc = rpcWrapperSupplier.get();`
	`244`	`+ } finally {`
	`245`	`+ _channelLock.unlock();`
`237`	`246`	`}`
`238`	`247`	`}`
`239`	`248`
`240`	`249`	`public boolean isOutstandingRpc()`
`241`	`250`	`{`
`242`		`- synchronized (_channelMutex) {`
	`251`	`+ _channelLock.lock();`
	`252`	`+ try {`
`243`	`253`	`return (_activeRpc != null);`
	`254`	`+ } finally {`
	`255`	`+ _channelLock.unlock();`
`244`	`256`	`}`
`245`	`257`	`}`
`246`	`258`
`247`	`259`	`public RpcWrapper nextOutstandingRpc()`
`248`	`260`	`{`
`249`		`- synchronized (_channelMutex) {`
	`261`	`+ _channelLock.lock();`
	`262`	`+ try {`
`250`	`263`	`RpcWrapper result = _activeRpc;`
`251`	`264`	`_activeRpc = null;`
`252`		`- _channelMutex.notifyAll();`
	`265`	`+ _channelLockCondition.signalAll();`
`253`	`266`	`return result;`
	`267`	`+ } finally {`
	`268`	`+ _channelLock.unlock();`
`254`	`269`	`}`
`255`	`270`	`}`
`256`	`271`
`@@ -344,36 +359,48 @@ private AMQCommand privateRpc(Method m, int timeout)`
`344`	`359`	`public void rpc(Method m, RpcContinuation k)`
`345`	`360`	`throws IOException`
`346`	`361`	`{`
`347`		`- synchronized (_channelMutex) {`
	`362`	`+ _channelLock.lock();`
	`363`	`+ try {`
`348`	`364`	`ensureIsOpen();`
`349`	`365`	`quiescingRpc(m, k);`
	`366`	`+ } finally {`
	`367`	`+ _channelLock.unlock();`
`350`	`368`	`}`
`351`	`369`	`}`
`352`	`370`
`353`	`371`	`public void quiescingRpc(Method m, RpcContinuation k)`
`354`	`372`	`throws IOException`
`355`	`373`	`{`
`356`		`- synchronized (_channelMutex) {`
	`374`	`+ _channelLock.lock();`
	`375`	`+ try {`
`357`	`376`	`enqueueRpc(k);`
`358`	`377`	`quiescingTransmit(m);`
	`378`	`+ } finally {`
	`379`	`+ _channelLock.unlock();`
`359`	`380`	`}`
`360`	`381`	`}`
`361`	`382`
`362`	`383`	`public void asyncRpc(Method m, CompletableFuture<Command> future)`
`363`	`384`	`throws IOException`
`364`	`385`	`{`
`365`		`- synchronized (_channelMutex) {`
	`386`	`+ _channelLock.lock();`
	`387`	`+ try {`
`366`	`388`	`ensureIsOpen();`
`367`	`389`	`quiescingAsyncRpc(m, future);`
	`390`	`+ } finally {`
	`391`	`+ _channelLock.unlock();`
`368`	`392`	`}`
`369`	`393`	`}`
`370`	`394`
`371`	`395`	`public void quiescingAsyncRpc(Method m, CompletableFuture<Command> future)`
`372`	`396`	`throws IOException`
`373`	`397`	`{`
`374`		`- synchronized (_channelMutex) {`
	`398`	`+ _channelLock.lock();`
	`399`	`+ try {`
`375`	`400`	`enqueueAsyncRpc(m, future);`
`376`	`401`	`quiescingTransmit(m);`
	`402`	`+ } finally {`
	`403`	`+ _channelLock.unlock();`
`377`	`404`	`}`
`378`	`405`	`}`
`379`	`406`
`@@ -402,13 +429,16 @@ public void processShutdownSignal(ShutdownSignalException signal,`
`402`	`429`	`boolean ignoreClosed,`
`403`	`430`	`boolean notifyRpc) {`
`404`	`431`	`try {`
`405`		`- synchronized (_channelMutex) {`
	`432`	`+ _channelLock.lock();`
	`433`	`+ try {`
`406`	`434`	`if (!setShutdownCauseIfOpen(signal)) {`
`407`	`435`	`if (!ignoreClosed)`
`408`	`436`	`throw new AlreadyClosedException(getCloseReason());`
`409`	`437`	`}`
`410`	`438`
`411`		`- _channelMutex.notifyAll();`
	`439`	`+ _channelLockCondition.signalAll();`
	`440`	`+ } finally {`
	`441`	`+ _channelLock.unlock();`
`412`	`442`	`}`
`413`	`443`	`} finally {`
`414`	`444`	`if (notifyRpc)`
`@@ -424,30 +454,40 @@ public void notifyOutstandingRpc(ShutdownSignalException signal) {`
`424`	`454`	`}`
`425`	`455`
`426`	`456`	`public void transmit(Method m) throws IOException {`
`427`		`- synchronized (_channelMutex) {`
	`457`	`+ _channelLock.lock();`
	`458`	`+ try {`
`428`	`459`	`transmit(new AMQCommand(m));`
	`460`	`+ } finally {`
	`461`	`+ _channelLock.unlock();`
`429`	`462`	`}`
`430`	`463`	`}`
`431`	`464`
`432`	`465`	`public void transmit(AMQCommand c) throws IOException {`
`433`		`- synchronized (_channelMutex) {`
	`466`	`+ _channelLock.lock();`
	`467`	`+ try {`
`434`	`468`	`ensureIsOpen();`
`435`	`469`	`quiescingTransmit(c);`
	`470`	`+ } finally {`
	`471`	`+ _channelLock.unlock();`
`436`	`472`	`}`
`437`	`473`	`}`
`438`	`474`
`439`	`475`	`public void quiescingTransmit(Method m) throws IOException {`
`440`		`- synchronized (_channelMutex) {`
	`476`	`+ _channelLock.lock();`
	`477`	`+ try {`
`441`	`478`	`quiescingTransmit(new AMQCommand(m));`
	`479`	`+ } finally {`
	`480`	`+ _channelLock.unlock();`
`442`	`481`	`}`
`443`	`482`	`}`
`444`	`483`
`445`	`484`	`public void quiescingTransmit(AMQCommand c) throws IOException {`
`446`		`- synchronized (_channelMutex) {`
	`485`	`+ _channelLock.lock();`
	`486`	`+ try {`
`447`	`487`	`if (c.getMethod().hasContent()) {`
`448`	`488`	`while (_blockContent) {`
`449`	`489`	`try {`
`450`		`- _channelMutex.wait();`
	`490`	`+ _channelLockCondition.await();`
`451`	`491`	`} catch (InterruptedException ignored) {`
`452`	`492`	`Thread.currentThread().interrupt();`
`453`	`493`	`}`
`@@ -460,6 +500,8 @@ public void quiescingTransmit(AMQCommand c) throws IOException {`
`460`	`500`	`}`
`461`	`501`	`this._trafficListener.write(c);`
`462`	`502`	`c.transmit(this);`
	`503`	`+ } finally {`
	`504`	`+ _channelLock.unlock();`
`463`	`505`	`}`
`464`	`506`	`}`
`465`	`507`