Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tomcat TEXT_FULL_WRITING state exception while multiple messages sent from server to client #632

Closed
ivy-lli opened this issue Jun 2, 2022 · 4 comments · Fixed by #638
Closed
Milestone

Comments

@ivy-lli
Copy link
Contributor

ivy-lli commented Jun 2, 2022

Hi

We use GLSP to display a diagram. GLSP itself uses LSP (LSP4J) as communication layer between server and client.
As our application already has a Tomcat web server we wanted to reuse (same port etc.) this one instead of provide something new like Jetty.

But now we run into many exceptions in our log, that a message couldn't be sent to the client:

[WARN ][org.eclipse.lsp4j.jsonrpc.RemoteEndpoint][IvyGlspActionExecutor-1]{}
Failed to send notification message.
java.lang.IllegalStateException: The remote endpoint was in state [TEXT_FULL_WRITING] which is an invalid state for called method
	at org.apache.tomcat.websocket.WsRemoteEndpointImplBase$StateMachine.checkState(WsRemoteEndpointImplBase.java:1274)
	at org.apache.tomcat.websocket.WsRemoteEndpointImplBase$StateMachine.textStart(WsRemoteEndpointImplBase.java:1236)
	at org.apache.tomcat.websocket.WsRemoteEndpointImplBase.sendStringByCompletion(WsRemoteEndpointImplBase.java:213)
	at org.apache.tomcat.websocket.WsRemoteEndpointImplBase.sendStringByFuture(WsRemoteEndpointImplBase.java:201)
	at org.apache.tomcat.websocket.WsRemoteEndpointAsync.sendText(WsRemoteEndpointAsync.java:53)
	at org.eclipse.lsp4j.websocket.WebSocketMessageConsumer.sendMessage(WebSocketMessageConsumer.java:57)
	at org.eclipse.lsp4j.websocket.WebSocketMessageConsumer.consume(WebSocketMessageConsumer.java:47)
	at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.notify(RemoteEndpoint.java:126)
	at org.eclipse.lsp4j.jsonrpc.services.EndpointProxy.invoke(EndpointProxy.java:88)
	at com.sun.proxy.$Proxy102.process(Unknown Source)
	at org.eclipse.glsp.server.actions.ClientActionHandler.send(ClientActionHandler.java:58)
	at org.eclipse.glsp.server.actions.ClientActionHandler.execute(ClientActionHandler.java:52)
	at ch.ivyteam.glsp.core.action.IvyActionDispatcher.runAction(IvyActionDispatcher.java:110)
	at ch.ivyteam.glsp.core.action.IvyActionDispatcher.handleAction(IvyActionDispatcher.java:89)
	at ch.ivyteam.glsp.core.action.IvyActionDispatcher.dispatch(IvyActionDispatcher.java:56)
	at org.eclipse.glsp.server.features.core.model.RequestModelActionHandler.notifyFinishedLoading(RequestModelActionHandler.java:68)
	at org.eclipse.glsp.server.features.core.model.RequestModelActionHandler.executeAction(RequestModelActionHandler.java:54)
	at org.eclipse.glsp.server.features.core.model.RequestModelActionHandler.executeAction(RequestModelActionHandler.java:1)
	at org.eclipse.glsp.server.actions.AbstractActionHandler.execute(AbstractActionHandler.java:53)
	at ch.ivyteam.glsp.core.action.IvyActionDispatcher.runAction(IvyActionDispatcher.java:110)
	at ch.ivyteam.glsp.core.action.IvyActionDispatcher.handleAction(IvyActionDispatcher.java:89)
	at ch.ivyteam.glsp.core.action.IvyActionDispatcher.lambda$0(IvyActionDispatcher.java:70)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)

After some code investigations and research, this sometime happens when multiple messages are sent to the client at the same "time" (or better quickly after each other). It seems that LSP4J sends here messages without waiting if the last message sending is completed: WebSocketMessageConsumer#sendMessage
The sendText method would return a Future<Void> to check if the message is sent completely.

Tomcat will directly try to send those messages to the client without waiting, it only checks the state of the connection. And because of this the exception above appears:

I've also done some quick researches in Jetty and it seems that there is a queue implemented which handles this "multiple" messages internally (but I'm not sure if I'm right): https://github.com/eclipse/jetty.project/blob/jetty-9.4.x/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/io/FrameFlusher.java#L78

I'm also not sure what is defined by the WebSocket protocol or the Java API if this should be handled by the server or the protocol (https://tomcat.apache.org/tomcat-9.0-doc/web-socket-howto.html). But if I understood it correctly form an older Tomcat issue, sending multiple messages without waiting is not supported: https://bz.apache.org/bugzilla/show_bug.cgi?id=63931#c1

And I think there is also no way to detect it outside of LSP4J e.g. in the GLSPClient#process, as there is no return value or exception (as LSP4J catches the exception and simply log it: RemoteEndpoint#notify.

So in the end I think the WebSocketMessageConsumer#sendMessage should hold a queue of messages and only send a new one if the previous is sent completely.
Or did I understand something wrong? Thank for you help 🙂

Currently in charge:

  • LSP4J v0.12.0
  • Tomcat v9.0.63
@jonahgraham
Copy link
Contributor

@ivy-lli Sorry that no one has an answer on this. The core developers of LSP4J don't use websockets much and the websockets bundles have been provided primarily as a convenience for consumers to have that shared code here. If you have a suggested PR I can try to review it.

I am tagging some of the known websocket LSP4J users in case they have more insight - they may not have seen your original issue.

cc: @mitasov-ra @apupier @spoenemann

@apupier
Copy link
Contributor

apupier commented Jun 9, 2022

I am tagging some of the known websocket LSP4J users in case they have more insight - they may not have seen your original issue.
Effectively not noticed the issue :-)

I never seen the reported of error. But I used Websocket mostly in Proof of Concept so far.

@ivy-lli Your analysis seems to be consistent and good.

@spoenemann
Copy link
Contributor

The approach with a queue sounds good. But strange that the websocket implementation doesn't take care of it by itself.

@ivy-lli
Copy link
Contributor Author

ivy-lli commented Jun 14, 2022

@spoenemann I've found another bug request in the tomcat bugzilla archive about this topic: https://bz.apache.org/bugzilla/show_bug.cgi?id=56026

It seems that they strictly followed the spec and the spec don't seems to clearly define if the server or the application should handle this.

@pisv pisv closed this as completed in 19500c5 Jun 28, 2022
@pisv pisv added this to the v0.15.0 milestone Jun 28, 2022
@pisv pisv linked a pull request Jun 28, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants