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

GLSP Server Error when closing GLSP Eclipse Editor #556 #48

Merged
merged 1 commit into from
Mar 8, 2022

Conversation

jfaltermeier
Copy link
Contributor

@jfaltermeier jfaltermeier commented Feb 23, 2022

This is a proposed fix for eclipse-glsp/glsp#556

The idea behind the fix is to a) make access to the clientidToDiagramEditor Map thread safe and b) to turn off the ActionDispatcher when the editor is about to be closed. This way any actions that are still sent from the client are ignored after the editor was removed from the GLSPEditorRegistry.

@jfaltermeier
Copy link
Contributor Author

@tortmayr Could you please have a look at this fix? Is there a better way to dispose communication when the editor is closed?

@planger planger requested a review from tortmayr March 7, 2022 07:43
Copy link
Contributor

@tortmayr tortmayr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, in general the changes look good to me.
However, I would not just dispose the ActionDispatcher in the notifyAboutToBeDisposed method. The cleaner solution would be to correctly dispose the client session here using the ClientSessionManager:

   public void notifyAboutToBeDisposed() {
      // we are about to be disposed. don't send and accept actions anymore
      getClientSessionManager().thenAccept(sessionManager -> sessionManager.disposeClientSession(getClientId()));
   }

   protected CompletableFuture<ClientSessionManager> getClientSessionManager() {
      return getInstance(ClientSessionManager.class);
   }

This will dispose the action dispatcher and any other running threads (e.g. the ModelSourceWatcher)

* notify the GLSPDiagram about the fact that it will be disposed. Stop
handling actions from that point on by disposing the client session
* make access to clientidToDiagramEditor map synchronised in
GLSPEditorRegistry

Signed-off-by: Johannes Faltermeier <jfaltermeier@eclipsesource.com>
Copy link
Contributor

@tortmayr tortmayr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants