martin-g commented on a change in pull request #330:
URL: https://github.com/apache/tomcat/pull/330#discussion_r466241414
##########
File path: java/org/apache/tomcat/websocket/WsSession.java
##########
@@ -809,20 +825,36 @@ protected void updateLastActive() {
lastActive = System.currentTimeMillis();
}
+ protected void updateLastActiveRead() {
+ lastActiveRead = System.currentTimeMillis();
+ }
protected void checkExpiration() {
long timeout = maxIdleTimeout;
if (timeout < 1) {
return;
}
+ long currentTime = System.currentTimeMillis();
+ boolean isWriteTimeout = currentTime - lastActive > timeout;
+ boolean isReadTimeout = currentTime - lastActiveRead > timeout;
- if (System.currentTimeMillis() - lastActive > timeout) {
- String msg = sm.getString("wsSession.timeout", getId());
- if (log.isDebugEnabled()) {
- log.debug(msg);
- }
- doClose(new CloseReason(CloseCodes.GOING_AWAY, msg),
- new CloseReason(CloseCodes.CLOSED_ABNORMALLY, msg));
+ if (isWriteTimeout || isReadTimeout) {
+ if(closeOnIdle) {
+ String msg = sm.getString("wsSession.timeout", getId());
+ if (log.isDebugEnabled()) {
+ log.debug(msg);
+ }
+ doClose(new CloseReason(CloseCodes.GOING_AWAY, msg),
+ new CloseReason(CloseCodes.CLOSED_ABNORMALLY, msg));
+ } else {
+ String msg = sm.getString("wsSession.timeout", getId());
Review comment:
indentation is off
##########
File path: java/javax/websocket/OnIdleSession.java
##########
@@ -0,0 +1,12 @@
+package javax.websocket;
Review comment:
I'm afraid this is not part of WebSocket specification:
https://www.jcp.org/en/jsr/detail?id=356 and
https://download.oracle.com/otndocs/jcp/websocket-1_1-mrel-eval-spec/index.html
So you cannot add it to `javax.websocket` package
##########
File path: java/javax/websocket/Session.java
##########
@@ -77,6 +77,19 @@
*/
void setMaxIdleTimeout(long timeout);
+ /**
+ * Get the flag to decide whether the session will be closed if idle Time
is passed
+ * @return true if the session is to be closed on expire
+ * false if the session will remain open and an IdleSession event
is thrown instead
+ */
+ boolean getCloseOnIdleTimeout();
+
+ /**
+ * Get the flag to decide whether the session will be closed if idle Time
is passed
+ * @param clsoeOnIdleTimeout- true for closing the session on idle timeout
+ * false for keeping the session open and
throwing IdleSession event instead.
+ */
+ void setCloseOnIdleTimeout(boolean clsoeOnIdleTimeout);
Review comment:
typo in the parameter name
##########
File path: java/org/apache/tomcat/websocket/WsSession.java
##########
@@ -98,6 +99,8 @@
private volatile int maxTextMessageBufferSize =
Constants.DEFAULT_BUFFER_SIZE;
private volatile long maxIdleTimeout = 0;
private volatile long lastActive = System.currentTimeMillis();
+ private volatile long lastActiveRead = System.currentTimeMillis();
Review comment:
`lastRead` ?
##########
File path: java/org/apache/tomcat/websocket/WsSession.java
##########
@@ -98,6 +99,8 @@
private volatile int maxTextMessageBufferSize =
Constants.DEFAULT_BUFFER_SIZE;
private volatile long maxIdleTimeout = 0;
private volatile long lastActive = System.currentTimeMillis();
Review comment:
Probably this one should be renamed to `lastWrite` ?!
----------------------------------------------------------------
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.
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
