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

DiagnosticServer: Adding a workspace folder with the VS code client leads to server shutdown #2

Open
CSchoel opened this issue Jun 11, 2021 · 2 comments

Comments

@CSchoel
Copy link
Contributor

CSchoel commented Jun 11, 2021

Steps to reproduce:

  • Start DiagnosticServer
  • Start VS code client
  • Use command Mo|E: connect to connect to server
  • Use File -> Add folder to workspace...

Expected behavior:

Server should stay alive after a single workspace/didChangeWorkspaceFolders has been received.

Actual behavior:

Client requests server to shut down. Also two workspace/didChangeWorkspaceFolders events are sent, the second being empty.

Ideas:

This might have something to do with missing entries in the ServerCapabilities, but this is hard to pinpoint since no exception is thrown.

@CSchoel
Copy link
Contributor Author

CSchoel commented Jun 14, 2021

Hot damn, this was a difficult one! 🤯

As it turns out, it was not an error on the server side, but on the side of the client instead - although the solution does affect the server side. Long story short, I misunderstood the life cycle of a VS Code extension to be something like this:

activate()  // by user-issued command
...
doStuff()
doSomeOtherStuff()
...
deactivate() // by user-issued command

However, it is actually more like this:

activate() // by user-issued command
...
doStuff()
...
deactivate()   // re-load of extension due to fundamental configuration changes
activate()
...
doSomeOtherStuff()
...
deactivate() // by user-issued command

Changing from a clean workspace without any open folders to a single-folder workspace apparently required a reload of the extension (as does changing from single-folder to multi-folder setup).

This means that I need to

  1. find a way to reuse the existing connection to the server between reloads by storing the client variable in some global data store.
  2. ask the user again for a port when the extension is reloaded in order to have enough time to manually restart the server.
  3. change the server implementation so that the server immediately starts listening again after it has been shut down.
  4. or let the client take control of starting/stopping the server process.

Currently I lean towards option 2. This is totally ugly from the point of view of the extension user, but I think it allows us maximum flexibility for testing the server. For a finished product, I would opt for option 4, as I think that this is the way it is done by most other projects and it keeps both the client and server code as simple and straightforward as possible. The only downside is that the current state of the OMC (loaded models, Modelica path, command line options) will be lost, so the user should at least be informed/warned that a reload occurred.

@CSchoel
Copy link
Contributor Author

CSchoel commented Jun 14, 2021

Side note: If you should debug the client in the future, note that debugging does not work during a reload of the extension, even if you interrupt the deactivate() function. The debugger shortly jumps to the break point, but then immediately exits, because the extension itself is shut down. This has caused major headaches during the (non-)debugging of 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

No branches or pull requests

1 participant