On 14/10/2019 17:58, ma...@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> markt pushed a commit to branch 7.0.x
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
> commit fe65d4c1aac1759e99befe417afde51dded567b5
> Author: Mark Thomas <ma...@apache.org>
> AuthorDate: Mon Oct 14 17:51:56 2019 +0100
>
> Remove sync that triggers deadlock with BZ 63816 test case
Hmm.
The same deadlock exists with APR.
However, the sync appears to be required to prevent bug 49884.
It looks like I need to spend some more time with the async code. I'll
remove the sync for now on the grounds that a deadlock is worse than a
10s delay but my intention is that that is only a temporary fix.
Mark
>
> I haven't been able to re-create the test failure on Linux that
> promoted the adding of this sync. It is sufficiently long ago
> that other refactoring may have provided an alternative fix.
> ---
> java/org/apache/tomcat/util/net/JIoEndpoint.java | 52
> ++++++++++++------------
> 1 file changed, 25 insertions(+), 27 deletions(-)
>
> diff --git a/java/org/apache/tomcat/util/net/JIoEndpoint.java
> b/java/org/apache/tomcat/util/net/JIoEndpoint.java
> index 166640a..024de03 100644
> --- a/java/org/apache/tomcat/util/net/JIoEndpoint.java
> +++ b/java/org/apache/tomcat/util/net/JIoEndpoint.java
> @@ -559,33 +559,31 @@ public class JIoEndpoint extends
> AbstractEndpoint<Socket> {
> public void processSocketAsync(SocketWrapper<Socket> socket,
> SocketStatus status) {
> try {
> - synchronized (socket) {
> - if (waitingRequests.remove(socket)) {
> - SocketProcessor proc = new
> SocketProcessor(socket,status);
> - ClassLoader loader =
> Thread.currentThread().getContextClassLoader();
> - try {
> - //threads should not be created by the webapp
> classloader
> - if (Constants.IS_SECURITY_ENABLED) {
> - PrivilegedAction<Void> pa = new
> PrivilegedSetTccl(
> - getClass().getClassLoader());
> - AccessController.doPrivileged(pa);
> - } else {
> - Thread.currentThread().setContextClassLoader(
> - getClass().getClassLoader());
> - }
> - // During shutdown, executor may be null - avoid NPE
> - if (!running) {
> - return;
> - }
> - getExecutor().execute(proc);
> - //TODO gotta catch RejectedExecutionException and
> properly handle it
> - } finally {
> - if (Constants.IS_SECURITY_ENABLED) {
> - PrivilegedAction<Void> pa = new
> PrivilegedSetTccl(loader);
> - AccessController.doPrivileged(pa);
> - } else {
> -
> Thread.currentThread().setContextClassLoader(loader);
> - }
> + if (waitingRequests.remove(socket)) {
> + SocketProcessor proc = new SocketProcessor(socket,status);
> + ClassLoader loader =
> Thread.currentThread().getContextClassLoader();
> + try {
> + //threads should not be created by the webapp classloader
> + if (Constants.IS_SECURITY_ENABLED) {
> + PrivilegedAction<Void> pa = new PrivilegedSetTccl(
> + getClass().getClassLoader());
> + AccessController.doPrivileged(pa);
> + } else {
> + Thread.currentThread().setContextClassLoader(
> + getClass().getClassLoader());
> + }
> + // During shutdown, executor may be null - avoid NPE
> + if (!running) {
> + return;
> + }
> + getExecutor().execute(proc);
> + //TODO gotta catch RejectedExecutionException and
> properly handle it
> + } finally {
> + if (Constants.IS_SECURITY_ENABLED) {
> + PrivilegedAction<Void> pa = new
> PrivilegedSetTccl(loader);
> + AccessController.doPrivileged(pa);
> + } else {
> + Thread.currentThread().setContextClassLoader(loader);
> }
> }
> }
>
>
> ---------------------------------------------------------------------
> 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
