Am 25.02.2015 15:34, schrieb r...@apache.org:
Author: remm
Date: Wed Feb 25 14:34:47 2015
New Revision: 1662226

URL: http://svn.apache.org/r1662226
Log:
Add a hack to skip the initial write event since it is not really
useful for websockets. Looking at the CI history and the concurrency
results for write, the problems with TestWebSocketFrameClient may
originate from 1660609.

Modified:

tomcat/trunk/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java

Modified:
tomcat/trunk/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java?rev=1662226&r1=1662225&r2=1662226&view=diff
==============================================================================
---
tomcat/trunk/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java
(original)
+++
tomcat/trunk/java/org/apache/tomcat/websocket/server/WsRemoteEndpointImplServer.java
Wed Feb 25 14:34:47 2015
@@ -59,7 +59,7 @@ public class WsRemoteEndpointImplServer

     private volatile long timeoutExpiry = -1;
     private volatile boolean close;
-
+    private volatile boolean first = true;

     public WsRemoteEndpointImplServer(ServletInputStream sis,
ServletOutputStream sos,
             WsServerContainer serverContainer) {
@@ -87,51 +87,57 @@ public class WsRemoteEndpointImplServer


     public void onWritePossible(boolean useDispatch) {
-        if (buffers == null) {
- // Servlet 3.1 will call the write listener once even if nothing
-            // was written
-            return;
-        }
-        boolean complete = false;
-        try {
- // If this is false there will be a call back when it is true
-            while (sos.isReady()) {
-                complete = true;
-                for (ByteBuffer buffer : buffers) {
-                    if (buffer.hasRemaining()) {
-                        complete = false;
- sos.write(buffer.array(), buffer.arrayOffset(),
-                                buffer.limit());
-                        buffer.position(buffer.limit());
-                        break;
+        ByteBuffer[] buffers = this.buffers;
+        if (first) {
+            // Wait for the fist message to do something
fist => first

+            first = false;
This is not really threadsafe. What happens when more than one thread call this method while first==false? They could (probably really really rarely) happen to get into this if condition at the "same" time, set first to false and do nothing. My reading would be, that this should happen only for the first call. Maybe thread safety isn't an issue, but do we need volatile in that case?


+        } else {
+            if (buffers == null) {
+                // Servlet 3.1 will call the write listener once even
if nothing
+                // was written
+                return;
+            }
+            boolean complete = false;
+            try {
+ // If this is false there will be a call back when it is true
+                while (sos.isReady()) {
+                    complete = true;
+                    for (ByteBuffer buffer : buffers) {
+                        if (buffer.hasRemaining()) {
+                            complete = false;
+ sos.write(buffer.array(), buffer.arrayOffset(),
+                                    buffer.limit());
+                            buffer.position(buffer.limit());
+                            break;
+                        }
The switching of the variable complete from false to true to false makes me a bit dizzy. I would have put the for loop into a method and returned the complete-status from there.
But this is purely cosmetic.

Regards
 Felix

                     }
-                }
-                if (complete) {
-                    sos.flush();
-                    complete = sos.isReady();
                     if (complete) {
-                        wsWriteTimeout.unregister(this);
-                        clearHandler(null, useDispatch);
-                        if (close) {
-                            close();
+                        sos.flush();
+                        complete = sos.isReady();
+                        if (complete) {
+                            wsWriteTimeout.unregister(this);
+                            clearHandler(null, useDispatch);
+                            if (close) {
+                                close();
+                            }
                         }
+                        break;
                     }
-                    break;
                 }
+            } catch (IOException | IllegalStateException e) {
+                wsWriteTimeout.unregister(this);
+                clearHandler(e, useDispatch);
+                close();
             }
-        } catch (IOException | IllegalStateException e) {
-            wsWriteTimeout.unregister(this);
-            clearHandler(e, useDispatch);
-            close();
-        }

-        if (!complete) {
-            // Async write is in progress
-            long timeout = getSendTimeout();
-            if (timeout > 0) {
-                // Register with timeout thread
-                timeoutExpiry = timeout + System.currentTimeMillis();
-                wsWriteTimeout.register(this);
+            if (!complete) {
+                // Async write is in progress
+                long timeout = getSendTimeout();
+                if (timeout > 0) {
+                    // Register with timeout thread
+ timeoutExpiry = timeout + System.currentTimeMillis();
+                    wsWriteTimeout.register(this);
+                }
             }
         }
     }



---------------------------------------------------------------------
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

Reply via email to