On Thu, Jan 18, 2024 at 8:18 PM <ma...@apache.org> wrote:
>
> This is an automated email from the ASF dual-hosted git repository.
>
> markt pushed a commit to branch 10.1.x
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
>
> The following commit(s) were added to refs/heads/10.1.x by this push:
>      new 67638c17e7 Fix backport of BZ 66508 regression fix
> 67638c17e7 is described below
>
> commit 67638c17e7c19a0280ccafa340183fb179af92f5
> Author: Mark Thomas <ma...@apache.org>
> AuthorDate: Thu Jan 18 18:52:30 2024 +0000
>
>     Fix backport of BZ 66508 regression fix

I don't understand the differences introduced.
In SocketWrapperBase, the lock is not a ReentrantLock, but it still
cannot be anything else:
private final Lock lock = new ReentrantLock();

So couldn't the code be the same as in Tomcat 11 ? (with an extra
cast, but no need to check anything)

Rémy

> ---
>  .../server/WsRemoteEndpointImplServer.java         | 61 
> +++++++++++-----------
>  1 file changed, 31 insertions(+), 30 deletions(-)
>
> diff --git 
> a/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java 
> b/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java
> index d0cfef46ac..3be5d4f382 100644
> --- a/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java
> +++ b/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java
> @@ -107,42 +107,43 @@ public class WsRemoteEndpointImplServer extends 
> WsRemoteEndpointImplBase {
>           * Special handling is required only when all of the following are 
> true:
>           * - A close message is being sent
>           * - This thread currently holds the socketWrapper lock (i.e. the 
> thread is current processing a socket event)
> +         *
> +         * Special handling is only possible if the socketWrapper lock is a 
> ReentrantLock (it will be by default)
>           */
> -        if (!(opCode == Constants.OPCODE_CLOSE && 
> socketWrapper.getLock().isHeldByCurrentThread())) {
> -            // Skip special handling
> -            return super.acquireMessagePartInProgressSemaphore(opCode, 
> timeoutExpiry);
> -        }
> -
> -        int socketWrapperLockCount;
>          if (socketWrapper.getLock() instanceof ReentrantLock) {
> -            socketWrapperLockCount = ((ReentrantLock) 
> socketWrapper.getLock()).getHoldCount();
> -        } else {
> -            socketWrapperLockCount = 1;
> -        }
> -        while (!messagePartInProgress.tryAcquire()) {
> -            if (timeoutExpiry < System.currentTimeMillis()) {
> -                return false;
> -            }
> -            try {
> -                // Release control of the processor
> -                socketWrapper.setCurrentProcessor(connection);
> -                // Release the per socket lock(s)
> -                for (int i = 0; i < socketWrapperLockCount; i++) {
> -                    socketWrapper.getLock().unlock();
> -                }
> -                // Provide opportunity for another thread to obtain the 
> socketWrapper lock
> -                Thread.yield();
> -            } finally {
> -                // Re-obtain the per socket lock(s)
> -                for (int i = 0; i < socketWrapperLockCount; i++) {
> -                    socketWrapper.getLock().lock();
> +            ReentrantLock reentrantLock = (ReentrantLock) 
> socketWrapper.getLock();
> +            if (opCode == Constants.OPCODE_CLOSE && 
> reentrantLock.isHeldByCurrentThread()) {
> +                int socketWrapperLockCount = reentrantLock.getHoldCount();
> +
> +                while (!messagePartInProgress.tryAcquire()) {
> +                    if (timeoutExpiry < System.currentTimeMillis()) {
> +                        return false;
> +                    }
> +                    try {
> +                        // Release control of the processor
> +                        socketWrapper.setCurrentProcessor(connection);
> +                        // Release the per socket lock(s)
> +                        for (int i = 0; i < socketWrapperLockCount; i++) {
> +                            socketWrapper.getLock().unlock();
> +                        }
> +                        // Provide opportunity for another thread to obtain 
> the socketWrapper lock
> +                        Thread.yield();
> +                    } finally {
> +                        // Re-obtain the per socket lock(s)
> +                        for (int i = 0; i < socketWrapperLockCount; i++) {
> +                            socketWrapper.getLock().lock();
> +                        }
> +                        // Re-take control of the processor
> +                        socketWrapper.takeCurrentProcessor();
> +                    }
>                  }
> -                // Re-take control of the processor
> -                socketWrapper.takeCurrentProcessor();
> +
> +                return true;
>              }
>          }
>
> -        return true;
> +        // Skip special handling
> +        return super.acquireMessagePartInProgressSemaphore(opCode, 
> timeoutExpiry);
>      }
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to