Issue #7086 - fix issues when calling websocket demand #7088
Conversation
Signed-off-by: Lachlan Roberts <[email protected]>
…sed. Signed-off-by: Lachlan Roberts <[email protected]>
Signed-off-by: Lachlan Roberts <[email protected]>
Signed-off-by: Lachlan Roberts <[email protected]>
...t/websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/WebSocketOpenTest.java
Show resolved
Hide resolved
| // Can send/receive binary message successfully. | ||
| ByteBuffer binaryMessage = BufferUtil.toBuffer("hello world"); | ||
| session.getBasicRemote().sendBinary(binaryMessage); | ||
| assertThat(BINARY_MESSAGES.poll(5, TimeUnit.SECONDS), is(binaryMessage)); |
There was a problem hiding this comment.
This is so confusing. That is(binaryMessage) actually calls equals() is so weird. Why not using assertEquals()?
Also, why binaryMessage is not consumed after sendBinary()?!? This seems like a bug, I wonder how the test is passing?
There was a problem hiding this comment.
This is so confusing. That is(binaryMessage) actually calls equals() is so weird. Why not using assertEquals()?
I don't see this as confusing, this pattern is frequently used in other tests.
Also, why binaryMessage is not consumed after sendBinary()?!? This seems like a bug, I wonder how the test is passing?
This was an intentional change that was made. See issue #4341. @gregw has previously said that sending a frame should not modify the position/limit of the original buffer.
Signed-off-by: Lachlan Roberts <[email protected]>
Closes #7086
If the call to
CoreSession.demand(1)throws then the callback will be completed for success and failure. This seems to be the most likely cause for what we are seeing in #7086, and I have been able to reproduce the same error stack trace in a test.