On Fri, Feb 1, 2019 at 4:50 PM Mark Thomas <ma...@apache.org> wrote:

> On 01/02/2019 14:41, Rémy Maucherat wrote:
> > On Thu, Jan 31, 2019 at 8:09 PM Mark Thomas <ma...@apache.org> wrote:
> >
> >> On 31/01/2019 16:31, Mark Thomas wrote:
> >>> On 31/01/2019 15:58, Rémy Maucherat wrote:
> >>>> On Wed, Jan 30, 2019 at 8:37 PM Mark Thomas <ma...@apache.org> wrote:
>
>
> <snip/>
>
> >>>> I had four issues then that appear to be fixed (three fixes and one
> test
> >>>> gone, which was
> >>>> com.sun.ts.tests.websocket.ee
> >> .javax.websocket.handshakeresponse.getHeadersHasOriginTest).
> >>>> com/sun/ts/tests/websocket/ee/javax/websocket/session/setTimeout1Test
> >>>> doesn't pass for me at the moment, although you put in your wiki it is
> >>>> fixed.
> >>>
> >>> I'm not 100% sure on that. It might be intermittent. Once I'd been
> >>> through everything once, I was going to do a full run and see where
> >>> things stand.
> >>
> >> Got to the bottom of that one (and 2 others). It was related to how
> >> quickly we checked for session timeout. The default is every 10s which
> >> isn't fast enough for those tests. I've added a system property to
> >> reduce it to every second and that fixes them.
> >>
> >
> > Hum, right, I think I remember that one now. It was a while ago.
>
> Maybe not. Those two are failing consistently for me now.
>

Will have to look at it again then.


>
> >>
> >>> Having looked at some of the TCK tests I am more in favour of making
> >>> some of the changes I was previously opposed to. Again, I was planning
> >>> on coming back to this after I'd done a new full run.
> >>
> >> Starting the full run now...
> >>
> >> Assuming it takes as long to run as it did last time (~2 hours) I'll
> >> come back to this tomorrow.
> >>
> >
> > So it's all good sorted out now, except the concurrency problem. The spec
> > probably implied concurrency was "ok". Resolving this by waiting for the
> > right state works for me (unsurprisingly), I have a first patch.
>
> I was thinking along the same lines but I was worried about possible
> deadlocks.
>

Here's my patch anyway.

Index: java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java
===================================================================
--- java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java
(revision 1852757)
+++ java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java
(working copy)
@@ -1159,32 +1159,32 @@
         private State state = State.OPEN;

         public synchronized void streamStart() {
-            checkState(State.OPEN);
+            waitState(State.OPEN);
             state = State.STREAM_WRITING;
         }

         public synchronized void writeStart() {
-            checkState(State.OPEN);
+            waitState(State.OPEN);
             state = State.WRITER_WRITING;
         }

         public synchronized void binaryPartialStart() {
-            checkState(State.OPEN, State.BINARY_PARTIAL_READY);
+            waitState(State.OPEN, State.BINARY_PARTIAL_READY);
             state = State.BINARY_PARTIAL_WRITING;
         }

         public synchronized void binaryStart() {
-            checkState(State.OPEN);
+            waitState(State.OPEN);
             state = State.BINARY_FULL_WRITING;
         }

         public synchronized void textPartialStart() {
-            checkState(State.OPEN, State.TEXT_PARTIAL_READY);
+            waitState(State.OPEN, State.TEXT_PARTIAL_READY);
             state = State.TEXT_PARTIAL_WRITING;
         }

         public synchronized void textStart() {
-            checkState(State.OPEN);
+            waitState(State.OPEN);
             state = State.TEXT_FULL_WRITING;
         }

@@ -1213,6 +1213,23 @@
                             "BUG: This code should never be called");
                 }
             }
+            notify();
+        }
+
+        private void waitState(State... required) {
+            while (true) {
+                for (State state : required) {
+                    if (this.state == state) {
+                        return;
+                    }
+                }
+                try {
+                    // TODO: timeout and break loop after a while
+                    wait(1000);
+                } catch (InterruptedException e) {
+                    // Ignore
+                }
+            }
         }

         private void checkState(State... required) {


>
> > Note: I cannot edit the confluence, what did I do wrong ?
>
> Nothing. You weren't granted edit privs. I've just fixed that.
>

Ah, I thought anyone could edit it.

Rémy


>
> Mark
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>

Reply via email to