On Fri, Jun 28, 2019 at 10:12 AM <[email protected]> wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> olegk pushed a commit to branch HTTPCORE-581
> in repository https://gitbox.apache.org/repos/asf/httpcomponents-core.git
>
> commit cef0c10d969ef7627b034578cd85e7fd05c8bccd
> Author: Oleg Kalnichevski <[email protected]>
> AuthorDate: Thu Jun 27 17:22:55 2019 +0200
>
> HTTPCORE-581: Wrong deadline calculation for non-blocking i/o
> operations
> ---
> .../java/org/apache/hc/core5/benchmark/HttpBenchmark.java | 5 +++++
> .../org/apache/hc/core5/testing/nio/LoggingIOSession.java | 5 +++++
> .../main/java/org/apache/hc/core5/reactor/IOSession.java | 14
> ++++++++++++++
> .../java/org/apache/hc/core5/reactor/IOSessionImpl.java | 10 ++++++++++
> .../java/org/apache/hc/core5/reactor/InternalChannel.java | 4 ++--
> .../apache/hc/core5/reactor/InternalConnectChannel.java | 2 +-
> .../org/apache/hc/core5/reactor/InternalDataChannel.java | 5 +++++
> .../java/org/apache/hc/core5/reactor/ssl/SSLIOSession.java | 5 +++++
> 8 files changed, 47 insertions(+), 3 deletions(-)
>
> diff --git
> a/httpcore5-testing/src/main/java/org/apache/hc/core5/benchmark/HttpBenchmark.java
> b/httpcore5-testing/src/main/java/org/apache/hc/core5/benchmark/HttpBenchmark.java
> index d8c1910..819fe7b 100644
> ---
> a/httpcore5-testing/src/main/java/org/apache/hc/core5/benchmark/HttpBenchmark.java
> +++
> b/httpcore5-testing/src/main/java/org/apache/hc/core5/benchmark/HttpBenchmark.java
> @@ -309,6 +309,11 @@ public class HttpBenchmark {
> }
>
> @Override
> + public long getLastEventTime() {
> + return ioSession.getLastEventTime();
> + }
>
IMO, this kind of method name must carry a scale, for example
getLastEventTime() -> getLastEventTimeMillis(). Better yet, why not use a
TimeValue?
Gary
+
> + @Override
> public void updateReadTime() {
> ioSession.updateReadTime();
> }
> diff --git
> a/httpcore5-testing/src/main/java/org/apache/hc/core5/testing/nio/LoggingIOSession.java
> b/httpcore5-testing/src/main/java/org/apache/hc/core5/testing/nio/LoggingIOSession.java
> index 65acd74..41697f3 100644
> ---
> a/httpcore5-testing/src/main/java/org/apache/hc/core5/testing/nio/LoggingIOSession.java
> +++
> b/httpcore5-testing/src/main/java/org/apache/hc/core5/testing/nio/LoggingIOSession.java
> @@ -219,6 +219,11 @@ public class LoggingIOSession implements
> ProtocolIOSession {
> }
>
> @Override
> + public long getLastEventTime() {
> + return this.session.getLastEventTime();
> + }
> +
> + @Override
> public NamedEndpoint getInitialEndpoint() {
> return this.session.getInitialEndpoint();
> }
> diff --git
> a/httpcore5/src/main/java/org/apache/hc/core5/reactor/IOSession.java
> b/httpcore5/src/main/java/org/apache/hc/core5/reactor/IOSession.java
> index cdbcefd..82304d3 100644
> --- a/httpcore5/src/main/java/org/apache/hc/core5/reactor/IOSession.java
> +++ b/httpcore5/src/main/java/org/apache/hc/core5/reactor/IOSession.java
> @@ -178,6 +178,11 @@ public interface IOSession extends ModalCloseable,
> Identifiable {
> /**
> * Sets value of the socket timeout in milliseconds. The value of
> * {@code 0} signifies the session cannot time out.
> + * <p>
> + * Please note this operation may affect last read / last write time
> values.
> + *
> + * @see #getLastReadTime()
> + * @see #getLastWriteTime()
> *
> * @param timeout socket timeout.
> */
> @@ -198,6 +203,15 @@ public interface IOSession extends ModalCloseable,
> Identifiable {
> long getLastWriteTime();
>
> /**
> + * Returns timestamp of the last I/O event including socket timeout
> reset.
> + *
> + * @see #getSocketTimeout()
> + *
> + * @return timestamp.
> + */
> + long getLastEventTime();
> +
> + /**
> * Updates the timestamp of the last read event
> */
> void updateReadTime();
> diff --git
> a/httpcore5/src/main/java/org/apache/hc/core5/reactor/IOSessionImpl.java
> b/httpcore5/src/main/java/org/apache/hc/core5/reactor/IOSessionImpl.java
> index 1cd7566..ef7ea06 100644
> ---
> a/httpcore5/src/main/java/org/apache/hc/core5/reactor/IOSessionImpl.java
> +++
> b/httpcore5/src/main/java/org/apache/hc/core5/reactor/IOSessionImpl.java
> @@ -58,6 +58,7 @@ class IOSessionImpl implements IOSession {
> private volatile Timeout socketTimeout;
> private volatile long lastReadTime;
> private volatile long lastWriteTime;
> + private volatile long lastEventTime;
>
> /**
> * Creates new instance of IOSessionImpl.
> @@ -77,6 +78,7 @@ class IOSessionImpl implements IOSession {
> final long currentTimeMillis = System.currentTimeMillis();
> this.lastReadTime = currentTimeMillis;
> this.lastWriteTime = currentTimeMillis;
> + this.lastEventTime = currentTimeMillis;
> }
>
> @Override
> @@ -174,16 +176,19 @@ class IOSessionImpl implements IOSession {
> @Override
> public void setSocketTimeout(final Timeout timeout) {
> this.socketTimeout = Timeout.defaultsToDisabled(timeout);
> + this.lastEventTime = System.currentTimeMillis();
> }
>
> @Override
> public void updateReadTime() {
> lastReadTime = System.currentTimeMillis();
> + lastEventTime = lastReadTime;
> }
>
> @Override
> public void updateWriteTime() {
> lastWriteTime = System.currentTimeMillis();
> + lastEventTime = lastWriteTime;
> }
>
> @Override
> @@ -197,6 +202,11 @@ class IOSessionImpl implements IOSession {
> }
>
> @Override
> + public long getLastEventTime() {
> + return lastEventTime;
> + }
> +
> + @Override
> public void close() {
> if (this.status.compareAndSet(ACTIVE, CLOSED)) {
> this.key.cancel();
> diff --git
> a/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalChannel.java
> b/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalChannel.java
> index 0078c6c..ac2c2e7 100644
> ---
> a/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalChannel.java
> +++
> b/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalChannel.java
> @@ -44,7 +44,7 @@ abstract class InternalChannel implements ModalCloseable
> {
>
> abstract Timeout getTimeout();
>
> - abstract long getLastReadTime();
> + abstract long getLastEventTime();
>
> final void handleIOEvent(final int ops) {
> try {
> @@ -61,7 +61,7 @@ abstract class InternalChannel implements ModalCloseable
> {
> final Timeout timeout = getTimeout();
> if (!timeout.isDisabled()) {
> final long timeoutMillis = timeout.toMillis();
> - final long deadlineMillis = getLastReadTime() + timeoutMillis;
> + final long deadlineMillis = getLastEventTime() +
> timeoutMillis;
> if (currentTimeMillis > deadlineMillis) {
> try {
> onTimeout(timeout);
> diff --git
> a/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalConnectChannel.java
> b/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalConnectChannel.java
> index 323687f..53781c2 100644
> ---
> a/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalConnectChannel.java
> +++
> b/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalConnectChannel.java
> @@ -84,7 +84,7 @@ final class InternalConnectChannel extends
> InternalChannel {
> }
>
> @Override
> - long getLastReadTime() {
> + long getLastEventTime() {
> return creationTimeMillis;
> }
>
> diff --git
> a/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalDataChannel.java
> b/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalDataChannel.java
> index c2f78f5..f69c02c 100644
> ---
> a/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalDataChannel.java
> +++
> b/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalDataChannel.java
> @@ -394,6 +394,11 @@ final class InternalDataChannel extends
> InternalChannel implements ProtocolIOSes
> }
>
> @Override
> + public long getLastEventTime() {
> + return ioSession.getLastEventTime();
> + }
> +
> + @Override
> public String toString() {
> final SSLIOSession tlsSession = tlsSessionRef.get();
> if (tlsSession != null) {
> diff --git
> a/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSession.java
> b/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSession.java
> index 5c24137..3eb33c1 100644
> ---
> a/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSession.java
> +++
> b/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSession.java
> @@ -824,6 +824,11 @@ public class SSLIOSession implements IOSession {
> return this.session.getLastWriteTime();
> }
>
> + @Override
> + public long getLastEventTime() {
> + return this.session.getLastEventTime();
> + }
> +
> private static void formatOps(final StringBuilder buffer, final int
> ops) {
> if ((ops & SelectionKey.OP_READ) > 0) {
> buffer.append('r');
>
>
