You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2019/08/21 13:12:05 UTC

[httpcomponents-core] branch HTTPASYNC-152 updated (aa9d7cb -> 670d550)

This is an automated email from the ASF dual-hosted git repository.

olegk pushed a change to branch HTTPASYNC-152
in repository https://gitbox.apache.org/repos/asf/httpcomponents-core.git.


 discard aa9d7cb  HTTPASYNC-152: minimize possibility of a race condition when I/O reactor session request gets cancelled ot times out immediately after its creation
 discard 00b7ade  Use AtomicBoolean to keep completion state in SessionRequestImpl
     add 1cd1a6e  Add SOAP xml content type (#142, back-ported from master)
     new 27d61cc  Use AtomicBoolean to keep completion state in SessionRequestImpl
     new 670d550  HTTPASYNC-152: minimize possibility of a race condition when I/O reactor session request gets cancelled ot times out immediately after its creation

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (aa9d7cb)
            \
             N -- N -- N   refs/heads/HTTPASYNC-152 (670d550)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 httpcore/src/main/java/org/apache/http/entity/ContentType.java | 2 ++
 1 file changed, 2 insertions(+)


[httpcomponents-core] 01/02: Use AtomicBoolean to keep completion state in SessionRequestImpl

Posted by ol...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

olegk pushed a commit to branch HTTPASYNC-152
in repository https://gitbox.apache.org/repos/asf/httpcomponents-core.git

commit 27d61cc3b9354ee82f4934c490cfd437d318b64c
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Mon Aug 19 14:45:29 2019 +0200

    Use AtomicBoolean to keep completion state in SessionRequestImpl
---
 .../http/impl/nio/reactor/SessionRequestImpl.java  | 127 ++++++++++-----------
 1 file changed, 60 insertions(+), 67 deletions(-)

diff --git a/httpcore-nio/src/main/java/org/apache/http/impl/nio/reactor/SessionRequestImpl.java b/httpcore-nio/src/main/java/org/apache/http/impl/nio/reactor/SessionRequestImpl.java
index f9656b9..94e57c0 100644
--- a/httpcore-nio/src/main/java/org/apache/http/impl/nio/reactor/SessionRequestImpl.java
+++ b/httpcore-nio/src/main/java/org/apache/http/impl/nio/reactor/SessionRequestImpl.java
@@ -31,9 +31,10 @@ import java.io.IOException;
 import java.net.SocketAddress;
 import java.nio.channels.Channel;
 import java.nio.channels.SelectionKey;
+import java.util.concurrent.atomic.AtomicBoolean;
 
-import org.apache.http.annotation.ThreadingBehavior;
 import org.apache.http.annotation.Contract;
+import org.apache.http.annotation.ThreadingBehavior;
 import org.apache.http.nio.reactor.IOSession;
 import org.apache.http.nio.reactor.SessionRequest;
 import org.apache.http.nio.reactor.SessionRequestCallback;
@@ -47,13 +48,13 @@ import org.apache.http.util.Args;
 @Contract(threading = ThreadingBehavior.SAFE_CONDITIONAL)
 public class SessionRequestImpl implements SessionRequest {
 
-    private volatile boolean completed;
-    private volatile SelectionKey key;
-
     private final SocketAddress remoteAddress;
     private final SocketAddress localAddress;
     private final Object attachment;
     private final SessionRequestCallback callback;
+    private final AtomicBoolean completed;
+
+    private volatile SelectionKey key;
 
     private volatile int connectTimeout;
     private volatile IOSession session = null;
@@ -70,7 +71,7 @@ public class SessionRequestImpl implements SessionRequest {
         this.localAddress = localAddress;
         this.attachment = attachment;
         this.callback = callback;
-        this.connectTimeout = 0;
+        this.completed = new AtomicBoolean(false);
     }
 
     @Override
@@ -90,7 +91,7 @@ public class SessionRequestImpl implements SessionRequest {
 
     @Override
     public boolean isCompleted() {
-        return this.completed;
+        return this.completed.get();
     }
 
     protected void setKey(final SelectionKey key) {
@@ -99,11 +100,11 @@ public class SessionRequestImpl implements SessionRequest {
 
     @Override
     public void waitFor() throws InterruptedException {
-        if (this.completed) {
+        if (this.completed.get()) {
             return;
         }
         synchronized (this) {
-            while (!this.completed) {
+            while (!this.completed.get()) {
                 wait();
             }
         }
@@ -125,16 +126,14 @@ public class SessionRequestImpl implements SessionRequest {
 
     public void completed(final IOSession session) {
         Args.notNull(session, "Session");
-        if (this.completed) {
-            return;
-        }
-        this.completed = true;
-        synchronized (this) {
-            this.session = session;
-            if (this.callback != null) {
-                this.callback.completed(this);
+        if (this.completed.compareAndSet(false, true)) {
+            synchronized (this) {
+                this.session = session;
+                if (this.callback != null) {
+                    this.callback.completed(this);
+                }
+                notifyAll();
             }
-            notifyAll();
         }
     }
 
@@ -142,45 +141,41 @@ public class SessionRequestImpl implements SessionRequest {
         if (exception == null) {
             return;
         }
-        if (this.completed) {
-            return;
-        }
-        this.completed = true;
-        final SelectionKey key = this.key;
-        if (key != null) {
-            key.cancel();
-            final Channel channel = key.channel();
-            try {
-                channel.close();
-            } catch (final IOException ignore) {}
-        }
-        synchronized (this) {
-            this.exception = exception;
-            if (this.callback != null) {
-                this.callback.failed(this);
+        if (this.completed.compareAndSet(false, true)) {
+            final SelectionKey key = this.key;
+            if (key != null) {
+                key.cancel();
+                final Channel channel = key.channel();
+                try {
+                    channel.close();
+                } catch (final IOException ignore) {}
+            }
+            synchronized (this) {
+                this.exception = exception;
+                if (this.callback != null) {
+                    this.callback.failed(this);
+                }
+                notifyAll();
             }
-            notifyAll();
         }
     }
 
     public void timeout() {
-        if (this.completed) {
-            return;
-        }
-        this.completed = true;
-        final SelectionKey key = this.key;
-        if (key != null) {
-            key.cancel();
-            final Channel channel = key.channel();
-            if (channel.isOpen()) {
-                try {
-                    channel.close();
-                } catch (final IOException ignore) {}
+        if (this.completed.compareAndSet(false, true)) {
+            final SelectionKey key = this.key;
+            if (key != null) {
+                key.cancel();
+                final Channel channel = key.channel();
+                if (channel.isOpen()) {
+                    try {
+                        channel.close();
+                    } catch (final IOException ignore) {}
+                }
             }
-        }
-        synchronized (this) {
-            if (this.callback != null) {
-                this.callback.timeout(this);
+            synchronized (this) {
+                if (this.callback != null) {
+                    this.callback.timeout(this);
+                }
             }
         }
     }
@@ -203,25 +198,23 @@ public class SessionRequestImpl implements SessionRequest {
 
     @Override
     public void cancel() {
-        if (this.completed) {
-            return;
-        }
-        this.completed = true;
-        final SelectionKey key = this.key;
-        if (key != null) {
-            key.cancel();
-            final Channel channel = key.channel();
-            if (channel.isOpen()) {
-                try {
-                    channel.close();
-                } catch (final IOException ignore) {}
+        if (this.completed.compareAndSet(false, true)) {
+            final SelectionKey key = this.key;
+            if (key != null) {
+                key.cancel();
+                final Channel channel = key.channel();
+                if (channel.isOpen()) {
+                    try {
+                        channel.close();
+                    } catch (final IOException ignore) {}
+                }
             }
-        }
-        synchronized (this) {
-            if (this.callback != null) {
-                this.callback.cancelled(this);
+            synchronized (this) {
+                if (this.callback != null) {
+                    this.callback.cancelled(this);
+                }
+                notifyAll();
             }
-            notifyAll();
         }
     }
 


[httpcomponents-core] 02/02: HTTPASYNC-152: minimize possibility of a race condition when I/O reactor session request gets cancelled ot times out immediately after its creation

Posted by ol...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

olegk pushed a commit to branch HTTPASYNC-152
in repository https://gitbox.apache.org/repos/asf/httpcomponents-core.git

commit 670d5500128ab7fed2e66efa2a9a8e9d6834a61a
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Mon Aug 19 14:46:43 2019 +0200

    HTTPASYNC-152: minimize possibility of a race condition when I/O reactor session request gets cancelled ot times out immediately after its creation
---
 .../http/impl/nio/reactor/AbstractIOReactor.java   | 24 +++++++++++++++++-----
 .../http/impl/nio/reactor/BaseIOReactor.java       | 12 +++++------
 .../http/impl/nio/reactor/SessionRequestImpl.java  |  8 ++++++++
 3 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/httpcore-nio/src/main/java/org/apache/http/impl/nio/reactor/AbstractIOReactor.java b/httpcore-nio/src/main/java/org/apache/http/impl/nio/reactor/AbstractIOReactor.java
index b0c150b..d84faeb 100644
--- a/httpcore-nio/src/main/java/org/apache/http/impl/nio/reactor/AbstractIOReactor.java
+++ b/httpcore-nio/src/main/java/org/apache/http/impl/nio/reactor/AbstractIOReactor.java
@@ -419,14 +419,23 @@ public abstract class AbstractIOReactor implements IOReactor {
             }
             try {
                 this.sessions.add(session);
+                key.attach(session);
                 final SessionRequestImpl sessionRequest = entry.getSessionRequest();
                 if (sessionRequest != null) {
-                    sessionRequest.completed(session);
+                    if (!sessionRequest.isTerminated()) {
+                        sessionRequest.completed(session);
+                    }
+                    if (!sessionRequest.isTerminated()) {
+                        sessionCreated(key, session);
+                    }
+                    if (sessionRequest.isTerminated()) {
+                        throw new CancelledKeyException();
+                    }
+                } else {
+                    sessionCreated(key, session);
                 }
-                key.attach(session);
-                sessionCreated(key, session);
             } catch (final CancelledKeyException ex) {
-                queueClosedSession(session);
+                session.close();
                 key.attach(null);
             }
         }
@@ -489,7 +498,12 @@ public abstract class AbstractIOReactor implements IOReactor {
             final int timeout = session.getSocketTimeout();
             if (timeout > 0) {
                 if (session.getLastAccessTime() + timeout < now) {
-                    sessionTimedOut(session);
+                    try {
+                        sessionTimedOut(session);
+                    } catch (final CancelledKeyException ex) {
+                        session.close();
+                        key.attach(null);
+                    }
                 }
             }
         }
diff --git a/httpcore-nio/src/main/java/org/apache/http/impl/nio/reactor/BaseIOReactor.java b/httpcore-nio/src/main/java/org/apache/http/impl/nio/reactor/BaseIOReactor.java
index 32d37db..c9caef2 100644
--- a/httpcore-nio/src/main/java/org/apache/http/impl/nio/reactor/BaseIOReactor.java
+++ b/httpcore-nio/src/main/java/org/apache/http/impl/nio/reactor/BaseIOReactor.java
@@ -169,8 +169,7 @@ public class BaseIOReactor extends AbstractIOReactor {
                 this.bufferingSessions.add(session);
             }
         } catch (final CancelledKeyException ex) {
-            queueClosedSession(session);
-            key.attach(null);
+            throw ex;
         } catch (final RuntimeException ex) {
             handleRuntimeException(ex);
         }
@@ -187,8 +186,7 @@ public class BaseIOReactor extends AbstractIOReactor {
         try {
             this.eventDispatch.outputReady(session);
         } catch (final CancelledKeyException ex) {
-            queueClosedSession(session);
-            key.attach(null);
+            throw ex;
         } catch (final RuntimeException ex) {
             handleRuntimeException(ex);
         }
@@ -230,7 +228,7 @@ public class BaseIOReactor extends AbstractIOReactor {
                     }
                 } catch (final CancelledKeyException ex) {
                     it.remove();
-                    queueClosedSession(session);
+                    session.close();
                 } catch (final RuntimeException ex) {
                     handleRuntimeException(ex);
                 }
@@ -247,7 +245,7 @@ public class BaseIOReactor extends AbstractIOReactor {
         try {
             this.eventDispatch.connected(session);
         } catch (final CancelledKeyException ex) {
-            queueClosedSession(session);
+            throw ex;
         } catch (final RuntimeException ex) {
             handleRuntimeException(ex);
         }
@@ -262,7 +260,7 @@ public class BaseIOReactor extends AbstractIOReactor {
         try {
             this.eventDispatch.timeout(session);
         } catch (final CancelledKeyException ex) {
-            queueClosedSession(session);
+            throw ex;
         } catch (final RuntimeException ex) {
             handleRuntimeException(ex);
         }
diff --git a/httpcore-nio/src/main/java/org/apache/http/impl/nio/reactor/SessionRequestImpl.java b/httpcore-nio/src/main/java/org/apache/http/impl/nio/reactor/SessionRequestImpl.java
index 94e57c0..bb45315 100644
--- a/httpcore-nio/src/main/java/org/apache/http/impl/nio/reactor/SessionRequestImpl.java
+++ b/httpcore-nio/src/main/java/org/apache/http/impl/nio/reactor/SessionRequestImpl.java
@@ -56,6 +56,7 @@ public class SessionRequestImpl implements SessionRequest {
 
     private volatile SelectionKey key;
 
+    private volatile boolean terminated;
     private volatile int connectTimeout;
     private volatile IOSession session = null;
     private volatile IOException exception = null;
@@ -94,6 +95,10 @@ public class SessionRequestImpl implements SessionRequest {
         return this.completed.get();
     }
 
+    boolean isTerminated() {
+        return this.terminated;
+    }
+
     protected void setKey(final SelectionKey key) {
         this.key = key;
     }
@@ -142,6 +147,7 @@ public class SessionRequestImpl implements SessionRequest {
             return;
         }
         if (this.completed.compareAndSet(false, true)) {
+            this.terminated = true;
             final SelectionKey key = this.key;
             if (key != null) {
                 key.cancel();
@@ -162,6 +168,7 @@ public class SessionRequestImpl implements SessionRequest {
 
     public void timeout() {
         if (this.completed.compareAndSet(false, true)) {
+            this.terminated = true;
             final SelectionKey key = this.key;
             if (key != null) {
                 key.cancel();
@@ -199,6 +206,7 @@ public class SessionRequestImpl implements SessionRequest {
     @Override
     public void cancel() {
         if (this.completed.compareAndSet(false, true)) {
+            this.terminated = true;
             final SelectionKey key = this.key;
             if (key != null) {
                 key.cancel();