[tomcat] branch main updated: Fix checkstyle
This is an automated email from the ASF dual-hosted git repository. remm pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/main by this push: new 7dbba140fd Fix checkstyle 7dbba140fd is described below commit 7dbba140fd93d22af08decf2496edac5f73715f8 Author: remm AuthorDate: Tue Apr 25 09:06:21 2023 +0200 Fix checkstyle --- .../apache/tomcat/jdbc/pool/PoolPropertiesTest.java | 21 ++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java index b475d64f23..01843bd3e8 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java @@ -1,12 +1,27 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package org.apache.tomcat.jdbc.pool; +import java.util.ArrayList; +import java.util.List; import org.junit.Assert; import org.junit.Test; -import java.util.ArrayList; -import java.util.List; - public class PoolPropertiesTest { private static final String DEFAULT_USER = "username_def"; private static final String DEFAULT_PASSWD = "password_def"; - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 10.1.x updated: Fix checkstyle
This is an automated email from the ASF dual-hosted git repository. remm 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 926cf93b42 Fix checkstyle 926cf93b42 is described below commit 926cf93b4292969bbfac91e8ae56937cb2497931 Author: remm AuthorDate: Tue Apr 25 09:06:21 2023 +0200 Fix checkstyle --- .../apache/tomcat/jdbc/pool/PoolPropertiesTest.java | 21 ++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java index b475d64f23..01843bd3e8 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java @@ -1,12 +1,27 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package org.apache.tomcat.jdbc.pool; +import java.util.ArrayList; +import java.util.List; import org.junit.Assert; import org.junit.Test; -import java.util.ArrayList; -import java.util.List; - public class PoolPropertiesTest { private static final String DEFAULT_USER = "username_def"; private static final String DEFAULT_PASSWD = "password_def"; - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 9.0.x updated: Fix checkstyle
This is an automated email from the ASF dual-hosted git repository. remm pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/9.0.x by this push: new 8baca9ff8a Fix checkstyle 8baca9ff8a is described below commit 8baca9ff8ae856c51e68663ad154a4ab7f728fd8 Author: remm AuthorDate: Tue Apr 25 09:06:21 2023 +0200 Fix checkstyle --- .../apache/tomcat/jdbc/pool/PoolPropertiesTest.java | 21 ++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java index b475d64f23..01843bd3e8 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java @@ -1,12 +1,27 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package org.apache.tomcat.jdbc.pool; +import java.util.ArrayList; +import java.util.List; import org.junit.Assert; import org.junit.Test; -import java.util.ArrayList; -import java.util.List; - public class PoolPropertiesTest { private static final String DEFAULT_USER = "username_def"; private static final String DEFAULT_PASSWD = "password_def"; - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 8.5.x updated: Fix checkstyle
This is an automated email from the ASF dual-hosted git repository. remm pushed a commit to branch 8.5.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/8.5.x by this push: new 7a3a3a4cf5 Fix checkstyle 7a3a3a4cf5 is described below commit 7a3a3a4cf554f9c4536f6c8672021b1f9de72987 Author: remm AuthorDate: Tue Apr 25 09:06:21 2023 +0200 Fix checkstyle --- .../apache/tomcat/jdbc/pool/PoolPropertiesTest.java | 21 ++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java index b475d64f23..01843bd3e8 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java @@ -1,12 +1,27 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package org.apache.tomcat.jdbc.pool; +import java.util.ArrayList; +import java.util.List; import org.junit.Assert; import org.junit.Test; -import java.util.ArrayList; -import java.util.List; - public class PoolPropertiesTest { private static final String DEFAULT_USER = "username_def"; private static final String DEFAULT_PASSWD = "password_def"; - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat] branch main updated: Fix checkstyle
> On Apr 25, 2023, at 15:07, r...@apache.org wrote: > > This is an automated email from the ASF dual-hosted git repository. > > remm pushed a commit to branch main > in repository https://gitbox.apache.org/repos/asf/tomcat.git > > > The following commit(s) were added to refs/heads/main by this push: > new 7dbba140fd Fix checkstyle > 7dbba140fd is described below > > commit 7dbba140fd93d22af08decf2496edac5f73715f8 > Author: remm > AuthorDate: Tue Apr 25 09:06:21 2023 +0200 > >Fix checkstyle > --- > .../apache/tomcat/jdbc/pool/PoolPropertiesTest.java | 21 ++--- > 1 file changed, 18 insertions(+), 3 deletions(-) > > diff --git > a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java > > b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java > index b475d64f23..01843bd3e8 100644 > --- > a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java > +++ > b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java > @@ -1,12 +1,27 @@ > +/* > + * Licensed to the Apache Software Foundation (ASF) under one or more > + * contributor license agreements. See the NOTICE file distributed with > + * this work for additional information regarding copyright ownership. > + * The ASF licenses this file to You under the Apache License, Version 2.0 > + * (the "License"); you may not use this file except in compliance with > + * the License. You may obtain a copy of the License at > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ Oops, thanks, I didn’t notice that this was missing. Han > package org.apache.tomcat.jdbc.pool; > > +import java.util.ArrayList; > +import java.util.List; > > import org.junit.Assert; > import org.junit.Test; > > -import java.util.ArrayList; > -import java.util.List; > - > public class PoolPropertiesTest { > private static final String DEFAULT_USER = "username_def"; > private static final String DEFAULT_PASSWD = "password_def"; > > > - > 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
[Bug 66574] Deadlock in websocket code
https://bz.apache.org/bugzilla/show_bug.cgi?id=66574 --- Comment #6 from Mark Thomas --- This is related to one of the fixes for bug 66508. Specifically this change: https://github.com/apache/tomcat/commit/dccc2644ce701e88b152563473a350ec33a29a81 In short, the above change replaced the internal session state lock with the SocketWrapper lock. This removed the possibility of a deadlock caused by container and non-container threads obtaining the locks in different orders. Thread #824 is simulating blocking by calling Future.get() immediately after initiating the async action that created the Future. This thread is holding the SocketWrapper lock because it is a container thread responding to the receipt of a WebSocket message. Thread #844 is processing the async action initiated by thread #824. It is trying to close the WebSocket session. To do this it needs the SocketWrapper lock which is held by #824. Hence threads #824 and #844 are deadlocked. I have observed that simulating blocking often triggers deadlocks with Tomcat. This appears to be another instance of that but one where the locks participating in the deadlock are split across Tomcat and the application. I'm going to spend some time between now and the next round of releases looking at the way Tomcat uses locks - particularly in WebSocket - with a view to seeing if there is a different approach that is less susceptible to deadlock issues. -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat-maven-plugin] fage88 commented on pull request #28: PFCORE-7274 upgrade tomcat 7 version to 7.0.92
fage88 commented on PR #28: URL: https://github.com/apache/tomcat-maven-plugin/pull/28#issuecomment-1522064083 Closing this PR, we abandoned Tomcat -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat-maven-plugin] fage88 closed pull request #28: PFCORE-7274 upgrade tomcat 7 version to 7.0.92
fage88 closed pull request #28: PFCORE-7274 upgrade tomcat 7 version to 7.0.92 URL: https://github.com/apache/tomcat-maven-plugin/pull/28 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 66574] Deadlock in websocket code
https://bz.apache.org/bugzilla/show_bug.cgi?id=66574 --- Comment #7 from Mark Thomas --- I have a patch that addresses this issue by refactoring the session state so it uses an AtomicReference (with a couple of extra states) rather than a lock. Initial testing is positive but I am currently running a fuller set of tests including the Autobahn test suite. Assuming that all goes well, are you able to test a build of Tomcat with the changes included if I post a link to the dev build here? I assume you'd want a Tomcat 9.0.x build to test. -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 66574] Deadlock in websocket code
https://bz.apache.org/bugzilla/show_bug.cgi?id=66574 --- Comment #8 from Boris Petrov --- Great, thanks a lot for the quick fix! I'll try to test with whatever you give me, yes. 9.0.x, correct. Thanks! -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch main updated: Fix BZ 66574 - refactor close to avoid possible deadlock
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/main by this push: new 36f134465d Fix BZ 66574 - refactor close to avoid possible deadlock 36f134465d is described below commit 36f134465dea9e2d7edf23e1b0cf5d0f571032d3 Author: Mark Thomas AuthorDate: Tue Apr 25 21:09:30 2023 +0100 Fix BZ 66574 - refactor close to avoid possible deadlock https://bz.apache.org/bugzilla/show_bug.cgi?id=66574 --- java/org/apache/tomcat/websocket/WsSession.java | 124 webapps/docs/changelog.xml | 5 + 2 files changed, 69 insertions(+), 60 deletions(-) diff --git a/java/org/apache/tomcat/websocket/WsSession.java b/java/org/apache/tomcat/websocket/WsSession.java index 8aad5a70ff..b8fe3b1db4 100644 --- a/java/org/apache/tomcat/websocket/WsSession.java +++ b/java/org/apache/tomcat/websocket/WsSession.java @@ -28,7 +28,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; -import java.util.concurrent.locks.Lock; +import java.util.concurrent.atomic.AtomicReference; import javax.naming.NamingException; @@ -107,7 +107,7 @@ public class WsSession implements Session { // Expected to handle message types of only private volatile MessageHandler binaryMessageHandler = null; private volatile MessageHandler.Whole pongMessageHandler = null; -private volatile State state = State.OPEN; +private AtomicReference state = new AtomicReference<>(State.OPEN); private final Map userProperties = new ConcurrentHashMap<>(); private volatile int maxBinaryMessageBufferSize = Constants.DEFAULT_BUFFER_SIZE; private volatile int maxTextMessageBufferSize = Constants.DEFAULT_BUFFER_SIZE; @@ -459,12 +459,12 @@ public class WsSession implements Session { @Override public boolean isOpen() { -return state == State.OPEN; +return state.get() == State.OPEN; } public boolean isClosed() { -return state == State.CLOSED; +return state.get() == State.CLOSED; } @@ -564,46 +564,38 @@ public class WsSession implements Session { * @param closeSocketShould the socket be closed immediately rather than waiting for the server to respond */ public void doClose(CloseReason closeReasonMessage, CloseReason closeReasonLocal, boolean closeSocket) { -// Double-checked locking. OK because state is volatile -if (state != State.OPEN) { + +if (!state.compareAndSet(State.OPEN, State.OUTPUT_CLOSING)) { +// Close process has already been started. Don't start it again. return; } -wsRemoteEndpoint.getLock().lock(); -try { -if (state != State.OPEN) { -return; -} - -if (log.isDebugEnabled()) { -log.debug(sm.getString("wsSession.doClose", id)); -} +if (log.isDebugEnabled()) { +log.debug(sm.getString("wsSession.doClose", id)); +} -// This will trigger a flush of any batched messages. -try { -wsRemoteEndpoint.setBatchingAllowed(false); -} catch (IOException e) { -log.warn(sm.getString("wsSession.flushFailOnClose"), e); -fireEndpointOnError(e); -} +// Flush any batched messages not yet sent. +try { +wsRemoteEndpoint.setBatchingAllowed(false); +} catch (IOException e) { +log.warn(sm.getString("wsSession.flushFailOnClose"), e); +fireEndpointOnError(e); +} +// Send the close message to the remote endpoint. +sendCloseMessage(closeReasonMessage); +fireEndpointOnClose(closeReasonLocal); +if (!state.compareAndSet(State.OUTPUT_CLOSING, State.OUTPUT_CLOSED) || closeSocket) { /* - * If the flush above fails the error handling could call this method recursively. Without this check, the - * close message and notifications could be sent multiple times. + * A close message was received in another thread or this is handling an error condition. Either way, no + * further close message is expected to be received. Mark the session as fully closed... */ -if (state != State.OUTPUT_CLOSED) { -state = State.OUTPUT_CLOSED; - -sendCloseMessage(closeReasonMessage); -if (closeSocket) { -wsRemoteEndpoint.close(); -} -fireEndpointOnClose(closeReasonLocal); -} -} finally { -wsRemoteEndpoint.getLock().unlock(); +state.set(State.CLOSED); +// ... and c
[tomcat] branch 10.1.x updated: Fix BZ 66574 - refactor close to avoid possible deadlock
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 260f5982ae Fix BZ 66574 - refactor close to avoid possible deadlock 260f5982ae is described below commit 260f5982aef0bb29180b9e0c5e9a60862e8ca1c7 Author: Mark Thomas AuthorDate: Tue Apr 25 21:09:30 2023 +0100 Fix BZ 66574 - refactor close to avoid possible deadlock https://bz.apache.org/bugzilla/show_bug.cgi?id=66574 --- java/org/apache/tomcat/websocket/WsSession.java | 124 webapps/docs/changelog.xml | 5 + 2 files changed, 69 insertions(+), 60 deletions(-) diff --git a/java/org/apache/tomcat/websocket/WsSession.java b/java/org/apache/tomcat/websocket/WsSession.java index 8aad5a70ff..b8fe3b1db4 100644 --- a/java/org/apache/tomcat/websocket/WsSession.java +++ b/java/org/apache/tomcat/websocket/WsSession.java @@ -28,7 +28,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; -import java.util.concurrent.locks.Lock; +import java.util.concurrent.atomic.AtomicReference; import javax.naming.NamingException; @@ -107,7 +107,7 @@ public class WsSession implements Session { // Expected to handle message types of only private volatile MessageHandler binaryMessageHandler = null; private volatile MessageHandler.Whole pongMessageHandler = null; -private volatile State state = State.OPEN; +private AtomicReference state = new AtomicReference<>(State.OPEN); private final Map userProperties = new ConcurrentHashMap<>(); private volatile int maxBinaryMessageBufferSize = Constants.DEFAULT_BUFFER_SIZE; private volatile int maxTextMessageBufferSize = Constants.DEFAULT_BUFFER_SIZE; @@ -459,12 +459,12 @@ public class WsSession implements Session { @Override public boolean isOpen() { -return state == State.OPEN; +return state.get() == State.OPEN; } public boolean isClosed() { -return state == State.CLOSED; +return state.get() == State.CLOSED; } @@ -564,46 +564,38 @@ public class WsSession implements Session { * @param closeSocketShould the socket be closed immediately rather than waiting for the server to respond */ public void doClose(CloseReason closeReasonMessage, CloseReason closeReasonLocal, boolean closeSocket) { -// Double-checked locking. OK because state is volatile -if (state != State.OPEN) { + +if (!state.compareAndSet(State.OPEN, State.OUTPUT_CLOSING)) { +// Close process has already been started. Don't start it again. return; } -wsRemoteEndpoint.getLock().lock(); -try { -if (state != State.OPEN) { -return; -} - -if (log.isDebugEnabled()) { -log.debug(sm.getString("wsSession.doClose", id)); -} +if (log.isDebugEnabled()) { +log.debug(sm.getString("wsSession.doClose", id)); +} -// This will trigger a flush of any batched messages. -try { -wsRemoteEndpoint.setBatchingAllowed(false); -} catch (IOException e) { -log.warn(sm.getString("wsSession.flushFailOnClose"), e); -fireEndpointOnError(e); -} +// Flush any batched messages not yet sent. +try { +wsRemoteEndpoint.setBatchingAllowed(false); +} catch (IOException e) { +log.warn(sm.getString("wsSession.flushFailOnClose"), e); +fireEndpointOnError(e); +} +// Send the close message to the remote endpoint. +sendCloseMessage(closeReasonMessage); +fireEndpointOnClose(closeReasonLocal); +if (!state.compareAndSet(State.OUTPUT_CLOSING, State.OUTPUT_CLOSED) || closeSocket) { /* - * If the flush above fails the error handling could call this method recursively. Without this check, the - * close message and notifications could be sent multiple times. + * A close message was received in another thread or this is handling an error condition. Either way, no + * further close message is expected to be received. Mark the session as fully closed... */ -if (state != State.OUTPUT_CLOSED) { -state = State.OUTPUT_CLOSED; - -sendCloseMessage(closeReasonMessage); -if (closeSocket) { -wsRemoteEndpoint.close(); -} -fireEndpointOnClose(closeReasonLocal); -} -} finally { -wsRemoteEndpoint.getLock().unlock(); +state.set(State.CLOSED); +// ... a
[tomcat] branch 9.0.x updated: Fix BZ 66574 - refactor close to avoid possible deadlock
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/9.0.x by this push: new 0046da8747 Fix BZ 66574 - refactor close to avoid possible deadlock 0046da8747 is described below commit 0046da8747927806c351e0032a7caaabf91c042e Author: Mark Thomas AuthorDate: Tue Apr 25 21:09:30 2023 +0100 Fix BZ 66574 - refactor close to avoid possible deadlock https://bz.apache.org/bugzilla/show_bug.cgi?id=66574 --- java/org/apache/tomcat/websocket/WsSession.java | 122 +--- webapps/docs/changelog.xml | 5 + 2 files changed, 69 insertions(+), 58 deletions(-) diff --git a/java/org/apache/tomcat/websocket/WsSession.java b/java/org/apache/tomcat/websocket/WsSession.java index 890f4f9d8a..18d7704f39 100644 --- a/java/org/apache/tomcat/websocket/WsSession.java +++ b/java/org/apache/tomcat/websocket/WsSession.java @@ -28,6 +28,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; import javax.naming.NamingException; import javax.websocket.ClientEndpointConfig; @@ -105,7 +106,7 @@ public class WsSession implements Session { // Expected to handle message types of only private volatile MessageHandler binaryMessageHandler = null; private volatile MessageHandler.Whole pongMessageHandler = null; -private volatile State state = State.OPEN; +private AtomicReference state = new AtomicReference<>(State.OPEN); private final Map userProperties = new ConcurrentHashMap<>(); private volatile int maxBinaryMessageBufferSize = Constants.DEFAULT_BUFFER_SIZE; private volatile int maxTextMessageBufferSize = Constants.DEFAULT_BUFFER_SIZE; @@ -541,12 +542,12 @@ public class WsSession implements Session { @Override public boolean isOpen() { -return state == State.OPEN; +return state.get() == State.OPEN; } public boolean isClosed() { -return state == State.CLOSED; +return state.get() == State.CLOSED; } @@ -646,46 +647,38 @@ public class WsSession implements Session { * @param closeSocketShould the socket be closed immediately rather than waiting for the server to respond */ public void doClose(CloseReason closeReasonMessage, CloseReason closeReasonLocal, boolean closeSocket) { -// Double-checked locking. OK because state is volatile -if (state != State.OPEN) { + +if (!state.compareAndSet(State.OPEN, State.OUTPUT_CLOSING)) { +// Close process has already been started. Don't start it again. return; } -wsRemoteEndpoint.getLock().lock(); -try { -if (state != State.OPEN) { -return; -} - -if (log.isDebugEnabled()) { -log.debug(sm.getString("wsSession.doClose", id)); -} +if (log.isDebugEnabled()) { +log.debug(sm.getString("wsSession.doClose", id)); +} -// This will trigger a flush of any batched messages. -try { -wsRemoteEndpoint.setBatchingAllowed(false); -} catch (IOException e) { -log.warn(sm.getString("wsSession.flushFailOnClose"), e); -fireEndpointOnError(e); -} +// Flush any batched messages not yet sent. +try { +wsRemoteEndpoint.setBatchingAllowed(false); +} catch (IOException e) { +log.warn(sm.getString("wsSession.flushFailOnClose"), e); +fireEndpointOnError(e); +} +// Send the close message to the remote endpoint. +sendCloseMessage(closeReasonMessage); +fireEndpointOnClose(closeReasonLocal); +if (!state.compareAndSet(State.OUTPUT_CLOSING, State.OUTPUT_CLOSED) || closeSocket) { /* - * If the flush above fails the error handling could call this method recursively. Without this check, the - * close message and notifications could be sent multiple times. + * A close message was received in another thread or this is handling an error condition. Either way, no + * further close message is expected to be received. Mark the session as fully closed... */ -if (state != State.OUTPUT_CLOSED) { -state = State.OUTPUT_CLOSED; - -sendCloseMessage(closeReasonMessage); -if (closeSocket) { -wsRemoteEndpoint.close(); -} -fireEndpointOnClose(closeReasonLocal); -} -} finally { -wsRemoteEndpoint.getLock().unlock(); +state.set(State.CLOSED); +// ...
[tomcat] branch 8.5.x updated: Fix BZ 66574 - refactor close to avoid possible deadlock
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 8.5.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/8.5.x by this push: new 6759f9e92b Fix BZ 66574 - refactor close to avoid possible deadlock 6759f9e92b is described below commit 6759f9e92b112056ab19e6cfc2291ae3137eb1b6 Author: Mark Thomas AuthorDate: Tue Apr 25 21:09:30 2023 +0100 Fix BZ 66574 - refactor close to avoid possible deadlock https://bz.apache.org/bugzilla/show_bug.cgi?id=66574 --- java/org/apache/tomcat/websocket/WsSession.java | 122 +--- webapps/docs/changelog.xml | 5 + 2 files changed, 69 insertions(+), 58 deletions(-) diff --git a/java/org/apache/tomcat/websocket/WsSession.java b/java/org/apache/tomcat/websocket/WsSession.java index 890f4f9d8a..18d7704f39 100644 --- a/java/org/apache/tomcat/websocket/WsSession.java +++ b/java/org/apache/tomcat/websocket/WsSession.java @@ -28,6 +28,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; import javax.naming.NamingException; import javax.websocket.ClientEndpointConfig; @@ -105,7 +106,7 @@ public class WsSession implements Session { // Expected to handle message types of only private volatile MessageHandler binaryMessageHandler = null; private volatile MessageHandler.Whole pongMessageHandler = null; -private volatile State state = State.OPEN; +private AtomicReference state = new AtomicReference<>(State.OPEN); private final Map userProperties = new ConcurrentHashMap<>(); private volatile int maxBinaryMessageBufferSize = Constants.DEFAULT_BUFFER_SIZE; private volatile int maxTextMessageBufferSize = Constants.DEFAULT_BUFFER_SIZE; @@ -541,12 +542,12 @@ public class WsSession implements Session { @Override public boolean isOpen() { -return state == State.OPEN; +return state.get() == State.OPEN; } public boolean isClosed() { -return state == State.CLOSED; +return state.get() == State.CLOSED; } @@ -646,46 +647,38 @@ public class WsSession implements Session { * @param closeSocketShould the socket be closed immediately rather than waiting for the server to respond */ public void doClose(CloseReason closeReasonMessage, CloseReason closeReasonLocal, boolean closeSocket) { -// Double-checked locking. OK because state is volatile -if (state != State.OPEN) { + +if (!state.compareAndSet(State.OPEN, State.OUTPUT_CLOSING)) { +// Close process has already been started. Don't start it again. return; } -wsRemoteEndpoint.getLock().lock(); -try { -if (state != State.OPEN) { -return; -} - -if (log.isDebugEnabled()) { -log.debug(sm.getString("wsSession.doClose", id)); -} +if (log.isDebugEnabled()) { +log.debug(sm.getString("wsSession.doClose", id)); +} -// This will trigger a flush of any batched messages. -try { -wsRemoteEndpoint.setBatchingAllowed(false); -} catch (IOException e) { -log.warn(sm.getString("wsSession.flushFailOnClose"), e); -fireEndpointOnError(e); -} +// Flush any batched messages not yet sent. +try { +wsRemoteEndpoint.setBatchingAllowed(false); +} catch (IOException e) { +log.warn(sm.getString("wsSession.flushFailOnClose"), e); +fireEndpointOnError(e); +} +// Send the close message to the remote endpoint. +sendCloseMessage(closeReasonMessage); +fireEndpointOnClose(closeReasonLocal); +if (!state.compareAndSet(State.OUTPUT_CLOSING, State.OUTPUT_CLOSED) || closeSocket) { /* - * If the flush above fails the error handling could call this method recursively. Without this check, the - * close message and notifications could be sent multiple times. + * A close message was received in another thread or this is handling an error condition. Either way, no + * further close message is expected to be received. Mark the session as fully closed... */ -if (state != State.OUTPUT_CLOSED) { -state = State.OUTPUT_CLOSED; - -sendCloseMessage(closeReasonMessage); -if (closeSocket) { -wsRemoteEndpoint.close(); -} -fireEndpointOnClose(closeReasonLocal); -} -} finally { -wsRemoteEndpoint.getLock().unlock(); +state.set(State.CLOSED); +// ...
[tomcat] branch main updated: Add release date for 11.0.0-M5
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/main by this push: new bd613e03ff Add release date for 11.0.0-M5 bd613e03ff is described below commit bd613e03fff8f88da40f758b67495f7091443908 Author: Mark Thomas AuthorDate: Tue Apr 25 21:21:28 2023 +0100 Add release date for 11.0.0-M5 --- webapps/docs/changelog.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index acf99d61c0..3e524e3fa0 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -135,7 +135,7 @@ - + - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
svn commit: r61474 - in /release/tomcat: tomcat-11/v11.0.0-M4/ tomcat-9/v9.0.73/
Author: markt Date: Tue Apr 25 20:24:25 2023 New Revision: 61474 Log: Drop older versions from CDN Removed: release/tomcat/tomcat-11/v11.0.0-M4/ release/tomcat/tomcat-9/v9.0.73/ - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch main updated: Fix warning
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/main by this push: new 800abedcbb Fix warning 800abedcbb is described below commit 800abedcbb75d853a4e044d1086548cafbcf0d84 Author: Mark Thomas AuthorDate: Tue Apr 25 21:26:22 2023 +0100 Fix warning --- .../src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java index 01843bd3e8..97f18fd0c3 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java @@ -42,7 +42,7 @@ public class PoolPropertiesTest { switch (c) { case '{': case '(': -case '[': stack.add(c); break; +case '[': stack.add(Character.valueOf(c)); break; case '}': Assert.assertEquals('{', stack.remove(stack.size() - 1).charValue()); break; case ')': Assert.assertEquals('(', stack.remove(stack.size() - 1).charValue()); break; case ']': Assert.assertEquals('[', stack.remove(stack.size() - 1).charValue()); break; - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 9.0.x updated: Fix warning
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/9.0.x by this push: new c8da1ade41 Fix warning c8da1ade41 is described below commit c8da1ade41164dcd4116da59ff362b895225f00f Author: Mark Thomas AuthorDate: Tue Apr 25 21:26:22 2023 +0100 Fix warning --- .../src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java index 01843bd3e8..97f18fd0c3 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java @@ -42,7 +42,7 @@ public class PoolPropertiesTest { switch (c) { case '{': case '(': -case '[': stack.add(c); break; +case '[': stack.add(Character.valueOf(c)); break; case '}': Assert.assertEquals('{', stack.remove(stack.size() - 1).charValue()); break; case ')': Assert.assertEquals('(', stack.remove(stack.size() - 1).charValue()); break; case ']': Assert.assertEquals('[', stack.remove(stack.size() - 1).charValue()); break; - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 10.1.x updated: Fix warning
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 c2d95eb783 Fix warning c2d95eb783 is described below commit c2d95eb783eb3a8f3dca6bcac46f6e68cfbd2557 Author: Mark Thomas AuthorDate: Tue Apr 25 21:26:22 2023 +0100 Fix warning --- .../src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java index 01843bd3e8..97f18fd0c3 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java @@ -42,7 +42,7 @@ public class PoolPropertiesTest { switch (c) { case '{': case '(': -case '[': stack.add(c); break; +case '[': stack.add(Character.valueOf(c)); break; case '}': Assert.assertEquals('{', stack.remove(stack.size() - 1).charValue()); break; case ')': Assert.assertEquals('(', stack.remove(stack.size() - 1).charValue()); break; case ']': Assert.assertEquals('[', stack.remove(stack.size() - 1).charValue()); break; - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 8.5.x updated: Fix warning
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 8.5.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/8.5.x by this push: new 9ddf8d73e3 Fix warning 9ddf8d73e3 is described below commit 9ddf8d73e35bdb07d2181325d4e27f86f6defc6f Author: Mark Thomas AuthorDate: Tue Apr 25 21:26:22 2023 +0100 Fix warning --- .../src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java index 01843bd3e8..97f18fd0c3 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/PoolPropertiesTest.java @@ -42,7 +42,7 @@ public class PoolPropertiesTest { switch (c) { case '{': case '(': -case '[': stack.add(c); break; +case '[': stack.add(Character.valueOf(c)); break; case '}': Assert.assertEquals('{', stack.remove(stack.size() - 1).charValue()); break; case ')': Assert.assertEquals('(', stack.remove(stack.size() - 1).charValue()); break; case ']': Assert.assertEquals('[', stack.remove(stack.size() - 1).charValue()); break; - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 66574] Deadlock in websocket code
https://bz.apache.org/bugzilla/show_bug.cgi?id=66574 --- Comment #9 from Mark Thomas --- Great. 9.0.75-dev build that includes the proposed fix for this deadlock is available at: https://people.apache.org/~markt/dev/v9.0.75-dev/ Usual caveats apply. This is not an official release. It is only intended to test the fix for this issue. Use at your own risk. -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Buildbot failure in on tomcat-11.0.x
Build status: BUILD FAILED: failed compile (failure) Worker used: bb_worker2_ubuntu URL: https://ci2.apache.org/#builders/112/builds/324 Blamelist: Mark Thomas Build Text: failed compile (failure) Status Detected: new failure Build Source Stamp: [branch main] 36f134465dea9e2d7edf23e1b0cf5d0f571032d3 Steps: worker_preparation: 0 git: 0 shell: 0 shell_1: 0 shell_2: 0 shell_3: 0 shell_4: 0 shell_5: 0 compile: 1 shell_6: 0 shell_7: 0 shell_8: 0 shell_9: 0 Rsync docs to nightlies.apache.org: 0 shell_10: 0 Rsync RAT to nightlies.apache.org: 0 compile_1: 2 shell_11: 0 Rsync Logs to nightlies.apache.org: 0 -- ASF Buildbot - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Buildbot success in on tomcat-11.0.x
Build status: Build succeeded! Worker used: bb_worker2_ubuntu URL: https://ci2.apache.org/#builders/112/builds/325 Blamelist: Mark Thomas Build Text: build successful Status Detected: restored build Build Source Stamp: [branch main] 800abedcbb75d853a4e044d1086548cafbcf0d84 Steps: worker_preparation: 0 git: 0 shell: 0 shell_1: 0 shell_2: 0 shell_3: 0 shell_4: 0 shell_5: 0 compile: 1 shell_6: 0 shell_7: 0 shell_8: 0 shell_9: 0 Rsync docs to nightlies.apache.org: 0 shell_10: 0 Rsync RAT to nightlies.apache.org: 0 compile_1: 1 shell_11: 0 Rsync Logs to nightlies.apache.org: 0 -- ASF Buildbot - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Buildbot failure in on tomcat-8.5.x
Build status: BUILD FAILED: failed compile (failure) Worker used: bb_worker2_ubuntu URL: https://ci2.apache.org/#builders/36/builds/462 Blamelist: Mark Thomas Build Text: failed compile (failure) Status Detected: new failure Build Source Stamp: [branch 8.5.x] 9ddf8d73e35bdb07d2181325d4e27f86f6defc6f Steps: worker_preparation: 0 git: 0 shell: 0 shell_1: 0 shell_2: 0 shell_3: 0 shell_4: 0 shell_5: 0 compile: 0 shell_6: 0 shell_7: 0 shell_8: 0 shell_9: 0 Rsync docs to nightlies.apache.org: 0 shell_10: 0 Rsync RAT to nightlies.apache.org: 0 compile_1: 2 shell_11: 0 Rsync Logs to nightlies.apache.org: 0 -- ASF Buildbot - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org