I went for c in the end. The TCK passes as do all of our own tests.

Back-porting the change is still on my personal TODO list.

Mark


On 05/02/2019 22:02, Romain Manni-Bucau wrote:
> As an user c sounds ok. Worse case maybe keep it for 1 release with some
> comm to have time to migrate if relevant.
> 
> Le mar. 5 févr. 2019 à 20:10, Rémy Maucherat <r...@apache.org> a écrit :
> 
>> On Tue, Feb 5, 2019 at 7:58 PM Mark Thomas <ma...@apache.org> wrote:
>>
>>> Hi,
>>>
>>> org.apache.tomcat.websocket.STREAMS_DROP_EMPTY_MESSAGES
>>>
>>> The above system property was added to address an issue with Tomcat's
>>> WebSocket implementation not passing the TCK. Tomcat was sending empty
>>> messages when the TCK wasn't expecting a message to be sent.
>>>
>>> Now that we have access to the TCK I have been able to track down the
>>> root cause of these failures.
>>>
>>> The root cause is this code in WsRemoteEndpointImplBase:
>>> ...
>>> } else if (encoder instanceof Encoder.TextStream) {
>>>     try (Writer w = getSendWriter()) {
>>>         ((Encoder.TextStream) encoder).encode(obj, w);
>>>     }
>>> }
>>>
>>> The call to getSendWriter() triggers the state change that a write has
>>> started. Then the exception is thrown and because of the
>>> try-with-resources close() is called. That triggers the end of message
>>> state change hence a zero length message is written.
>>>
>>> The STREAMS_DROP_EMPTY_MESSAGES causes all empty messages (not just in
>>> this error case) to be dropped.
>>>
>>> I'm currently thinking that handling of this error case and
>>> STREAMS_DROP_EMPTY_MESSAGES should be decoupled. The idea is that, in
>>> the above case, obtaining the Writer is delayed until the encoder tries
>>> to write bytes.
>>>
>>> If the above change is implemented then what should be happen to
>>> STREAMS_DROP_EMPTY_MESSAGES?
>>> a) keep it as is
>>> b) deprecate it and remove it in 10.x
>>> c) remove it now
>>>
>>> I'm leaning towards b)
>>>
>>
>> You could probably do c) as the only purpose was to stick to the TCK
>> behavior. When there is no spec, the TCK is supposed to be the referee (and
>> I don't really see why empty messages are useful).
>>
>> Rémy
>>
>>
>>>
>>> Thoughts?
>>>
>>> Mark
>>>
>>> ---------------------------------------------------------------------
>>> 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