On 24/02/2015 13:10, Rémy Maucherat wrote: > I'm having issues with the write timeout tests in > TestWsWebSocketContainer, which made me do some changes since there are > still things I don't understand:
These appear to be OK for me at the moment with NIO and NIO2 but the very nature of timing issues means that doesn't count for much. I am seeing failures or crashes with APR/native so there is still work to be done there. > - In WsRemoteEndpointImplServer, onWritePossible appears to be able to be > invoked concurrently (doWrite calls it directly and changes the buffers). I > think it should be synced. Those calls should be nested. If you are seeing concurrent calls then there is probably still an issue around write registration. > - In Nio2Endpoint socket wrapper uses nestedWriteCompletionCount over the > inline flag that was used in 8. If the write completes inline, then isReady > should already be set back to true, and writing could continue. So the > change was IMO adding more write notifications which could hide some > issues. I tried changing that many times following the refactoring started, > but this is the first time I can do it without obviously breaking the > testsuite (where some of the non blocking write tests would hang due to > missing write notifications). This change was to prevent multiple write threads being triggered if there were multiple levels of nesting with the write completion handler. It was a fairly rare event but it did happen. > - NPE guards in the NIO connector socket processor for concurrent closing > [NIO2 has them, somehow it wasn't needed earlier in NIO, which is also an > odd thing; I actually feel better having to add them]. > > So this could improve on some possible timing related problems. I'll keep > on investigating though before committing anything. One thing to keep in mind that may simplify some of these issues is that once WebSocket moves to using the Tomcat I/O layer directly the requirement for one container thread reading and one container thread writing concurrently will go away. A number of the concurrency issues we have observed are triggered by these concurrent threads so switching back to a single thread should help. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
