On 04/12/2019 07:55, Rémy Maucherat wrote:
> On Tue, Dec 3, 2019 at 10:56 PM Mark Thomas <ma...@apache.org
> <mailto:ma...@apache.org>> wrote:
> 
>     Hi,
> 
>     I've been looking into BZ 63931. The error messages I was expecting to
>     see were not there.
> 
>     The first odd thing I noticed was an NPE here:
>     
> https://github.com/apache/tomcat/blob/master/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java#L310
> 
>     bsh.getSendResult() returned null. That shouldn't happen.
> 
>     I added some debug logging and found that the call to writeMessagePart()
>     on the previous line was returning before the SendResult was populated.
> 
>     Tracing the code for writeMessagePart() led me to
>     SocketWrapperBase.vectoredOperation()
> 
>     The issue appears to be that state.start() at
> 
>     
> https://github.com/apache/tomcat/blob/master/java/org/apache/tomcat/util/net/SocketWrapperBase.java#L1425
> 
>     effectively always does non-blocking writes. If the write buffer is full
>     (which is the scenario I am testing) 0 bytes are written, the socket is
>     passed to the Poller and the call returns.
> 
>     I hacked in the following code just after the state.start()
> 
>     if (block == BlockingMode.BLOCK) {
>         if (read) {
>             readPending.acquireUninterruptibly();
>             readPending.release();
>         } else {
>             writePending.acquireUninterruptibly();
>             writePending.release();
>         }
>     }
> 
>     I'm sure that code is wrong. It was just intended as a quick hack to
>     force the call to block until the write had completed. It had the
>     desired effect and I started to see the error messages I expected.
> 
>     I'm not familiar enough with the asyncIO code yet to know where the best
>     place to fix this is. Rémy, are you able to help?
> 
> 
> I'm not sure about the problem. Is there a test case ?

Sort of. I've been using the following:

1. Configure TestWebSocketFrameClient.testConnectToServerEndpoint() to
   run from an IDE in debug mode.

2. Put a break point in
   TesterMessageCountClient.BasicText.onMessage(String) so the client
   hangs and doesn't process any messages (causing the network buffers
   to fill up).

3. In WsRemoteEndpointImplBase.sendMessageBlock() in the loop that
   writes each of the message parts add this to the start of the loop:
   if (bsh.getSendResult() == null) {
       System.out.println("Prepare for NPE");
   }
   and put a breakpoint on the System.out. Execution should never reach
   that line but it does.

> Blocking is also used for HTTP/2 where it seems to work. If the write
> buffer is full, 0 bytes are written and the socket is indeed passed to
> the poller but until the operation is complete it is supposed to be
> blocked here:
> https://github.com/apache/tomcat/blob/master/java/org/apache/tomcat/util/net/SocketWrapperBase.java#L1430

That helps a lot. A san try and see why things aren't blocking there.

> Since I made the fix here for NIO (
> https://github.com/apache/tomcat/commit/f5f2b62670f972fc6a857788084e4352f2d4cd87
> ) I may need to do the same loop here however (when writing the buffer
> it is there):
> https://github.com/apache/tomcat/blob/master/java/org/apache/tomcat/util/net/NioEndpoint.java#L1493

I see you committed that. I'll see what difference it makes.

Mark

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

Reply via email to