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.

Sign up for GitHub

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

Jump to bottom

Restarting LSP does not kill coq-lsp process #230

Closed
Alizter opened this issue Jan 24, 2023 · 2 comments · Fixed by #375, ocaml/opam-repository#23390 or ocaml/opam-repository#23614
Closed

Restarting LSP does not kill coq-lsp process #230

Alizter opened this issue Jan 24, 2023 · 2 comments · Fixed by #375, ocaml/opam-repository#23390 or ocaml/opam-repository#23614
Labels
kind: bug Something isn't working needs: reproduction method
Milestone
0.1.6

Comments

@Alizter
Copy link
Collaborator

Alizter commented Jan 24, 2023

After having restarted LSP several times, I noticed that the old processes were not being killed.
@ejgallego
Copy link
Owner

I've seen this only once, but likely my computer is too fast to reproduce a race here. There could be 2 reasons:

  • Node / vscode-languageclient problem: in this case the issue should be forward upstream detailing that the stop() method has this problem, c.f. https://github.com/microsoft/vscode-languageserver-node
  • The extension was disposed and indeed we didn't push the client to the extension subscriptions, this seems like a bug! Can we check what the reference extensions do here? But for sure something to try to see if that fixes your problem.

How often does this problem happen?

@ejgallego
Copy link
Owner

Indeed, @artagnon found that the exit procedure was totally broken, PR incoming, I bet that helps with this problem.

@ejgallego ejgallego added this to the 0.1.6 milestone Feb 17, 2023
ejgallego added a commit that referenced this issue Feb 17, 2023
@ejgallego @artagnon
[controller] Fix bug where the controller was not exiting
536c788 
Problem found by Ramkumar, great job!

Indeed I think we never had the right code handling this, as observed
in many instances when the server complained about a forced shutdown.

Likely fixes #230

Co-authored-by: Ramkumar Ramachandra <r@artagnon.com>
@ejgallego ejgallego mentioned this issue Feb 17, 2023
Merged
ejgallego added a commit that referenced this issue Feb 17, 2023
@ejgallego @artagnon
[controller] Fix bug where the controller was not exiting
4ca0f21 
Problem found by Ramkumar, great job!

Indeed I think we never had the right code handling this, as observed
in many instances when the server complained about a forced shutdown.

Likely fixes #230

Co-authored-by: Ramkumar Ramachandra <r@artagnon.com>
ejgallego added a commit that referenced this issue Feb 17, 2023
@ejgallego @artagnon
[controller] Fix bug where the controller was not exiting
7dd0628 
Problem found by Ramkumar, great job!

Indeed I think we never had the right code handling this, as observed
in many instances when the server complained about a forced shutdown.

Likely fixes #230

Co-authored-by: Ramkumar Ramachandra <r@artagnon.com>
ejgallego added a commit that referenced this issue Feb 17, 2023
@ejgallego @artagnon
[controller] Fix bug where the controller was not exiting
70a787b 
Problem found by Ramkumar, great job!

Indeed I think we never had the right code handling this, as observed
in many instances when the server complained about a forced shutdown.

Likely fixes #230

Co-authored-by: Ramkumar Ramachandra <r@artagnon.com>
ejgallego added a commit that referenced this issue Feb 17, 2023
@ejgallego @artagnon
[controller] Fix bug where the controller was not exiting
1451309 
Problem found by Ramkumar, great job!

Indeed I think we never had the right code handling this, as observed
in many instances when the server complained about a forced shutdown.

Likely fixes #230

Co-authored-by: Ramkumar Ramachandra <r@artagnon.com>
ejgallego added a commit that referenced this issue Feb 17, 2023
@ejgallego @artagnon
[controller] Fix bug where the controller was not exiting
edede78 
Problem found by Ramkumar, great job!

Indeed I think we never had the right code handling this, as observed
in many instances when the server complained about a forced shutdown.

Likely fixes #230

Co-authored-by: Ramkumar Ramachandra <r@artagnon.com>
ejgallego added a commit that referenced this issue Feb 17, 2023
@ejgallego @artagnon
[controller] Fix bug where the controller was not exiting
751a593 
Problem found by Ramkumar, great job!

Indeed I think we never had the right code handling this, as observed
in many instances when the server complained about a forced shutdown.

Likely fixes #230

Co-authored-by: Ramkumar Ramachandra <r@artagnon.com>
ejgallego added a commit that referenced this issue Feb 17, 2023
@ejgallego @artagnon
[controller] Fix bug where the controller was not exiting
3a5dedf 
Problem found by Ramkumar, great job!

Indeed I think we never had the right code handling this, as observed
in many instances when the server complained about a forced shutdown.

Likely fixes #230

Co-authored-by: Ramkumar Ramachandra <r@artagnon.com>
ejgallego added a commit that referenced this issue Feb 17, 2023
@ejgallego @artagnon
[controller] Fix bug where the controller was not exiting
0a4ccc1 
Problem found by Ramkumar, great job!

Indeed I think we never had the right code handling this, as observed
in many instances when the server complained about a forced shutdown.

Likely fixes #230

Co-authored-by: Ramkumar Ramachandra <r@artagnon.com>
ejgallego added a commit that referenced this issue Feb 17, 2023
@ejgallego @artagnon
[controller] Fix bug where the controller was not exiting
1a2d202 
Problem found by Ramkumar, great job!

Indeed I think we never had the right code handling this, as observed
in many instances when the server complained about a forced shutdown.

Likely fixes #230

Co-authored-by: Ramkumar Ramachandra <r@artagnon.com>
ejgallego added a commit that referenced this issue Feb 17, 2023
@ejgallego @artagnon
[controller] Fix bug where the controller was not exiting
6631546 
Problem found by Ramkumar, great job!

Indeed I think we never had the right code handling this, as observed
in many instances when the server complained about a forced shutdown.

Likely fixes #230

Co-authored-by: Ramkumar Ramachandra <r@artagnon.com>
@ejgallego ejgallego mentioned this issue Feb 22, 2023
Merged
ejgallego added a commit to ejgallego/opam-repository that referenced this issue Feb 22, 2023
@ejgallego
[new release] coq-lsp (0.1.6+8.16)
673cf97 
CHANGES:

---------------------

 - The info / goal view now uses jsCoq's client-side rendering, with
   better highlighting and layout rendering (@artagnon, @ejgallego,
   ejgallego/coq-lsp#143, fixes ejgallego/coq-lsp#96)
 - Printing method is now configurable by the user (@ejgallego, ejgallego/coq-lsp#143,
   fixes ejgallego/coq-lsp#321)
 - Trigger completion on quote char "'" (@ejgallego, ejgallego/coq-lsp#350)
 - Fix typo on keybinding config for show goals (@tomtomjhj, ejgallego/coq-lsp#357)
 - New request `coq/getDocument` to get serialized full document
   contents. Thanks to Clément Pit-Claudel for feedback and ideas.
   (@ejgallego, ejgallego/coq-lsp#350)
 - Auto-ignore Coq object files; can be disabled in config
   (@ejgallego, ejgallego/coq-lsp#365)
 - Support workspaces with multiple roots, this is very useful for
   projects that contain several `_CoqProject` files in different
   directories (@ejgallego, ejgallego/coq-lsp#374)
 - Add VS Code commands to start / stop the server (@ejgallego, ejgallego/coq-lsp#377,
   cc ejgallego/coq-lsp#209)
 - Fix bug that made the server not exit on `exit` LSP notification
   (@artagnon, @ejgallego, ejgallego/coq-lsp#375, fixes ejgallego/coq-lsp#230)
 - Lay the foundation for server tests (@artagnon, ejgallego/coq-lsp#356)
 - Remove the `coq-lsp.ok_diagnostics` setting (@artagnon, ejgallego/coq-lsp#129)
 - Print abbreviations on hover (@ejgallego, ejgallego/coq-lsp#384)
 - Print hover types without parenthesis (@ejgallego, ejgallego/coq-lsp#384)
 - Parse identifiers with dot for hover and jump to definition
   (@ejallego, ejgallego/coq-lsp#385)
 - Update `vscode-languageclient` to 8.1.0 (@ejgallego, @Alizter,
   ejgallego/coq-lsp#383, fixes ejgallego/coq-lsp#273)
 - Fix typo on max_errors checking, this made coq-lsp stop on the
   number of total diagnostics, instead of only errors (@ejgallego,
   ejgallego/coq-lsp#386)
 - Hover symbol information: hypothesis names must shadow globals of
   the same name (@ejgallego, ejgallego/coq-lsp#391, fixes ejgallego/coq-lsp#388)
 - De-schedule document on didClose, otherwise the scheduler will keep
   trying to resume it if it didn't finish (@ejgallego, ejgallego/coq-lsp#392)
 - Hover symbol information: correctly handle identifiers before '.'
   and containing a quote (') themselves (@ejgallego, ejgallego/coq-lsp#393)
 - Add children entries to the table-of-contents (@ejgallego, ejgallego/coq-lsp#394)
 - Invalidate Coq's imperative cache on error (@ejgallego, @r-muhairi,
   ejgallego/coq-lsp#395)
 - Add status bar button to toggle server run status (@ejgallego,
   @Alizter, ejgallego/coq-lsp#378, closes ejgallego/coq-lsp#209)
 - Support for `COQLIB` and `COQCORELIB` environment variables, added
   `--coqcorelib` command line argument (@ejgallego, ejgallego/coq-lsp#403)
 - Protocol infrastructure for code lenses (@ejgallego, ejgallego/coq-lsp#396)
 - Set binary more for protocol input / output (@ejgallego, ejgallego/coq-lsp#408)
 - Allow to set `ocamlpath` from the command line (@ejgallego, ejgallego/coq-lsp#408)
 - Windows support (@ejgallego, @jim-portegies, ejgallego/coq-lsp#408)
 - Scroll active goal into view (@ejgallego, ejgallego/coq-lsp#410, fixes ejgallego/coq-lsp#381)
 - Server status icon will now react properly to fatal server errors
   (@ejgallego, reported by @Alizter, ejgallego/coq-lsp#411, fixes ejgallego/coq-lsp#399)
 - Info on memory and time is now disabled by default, new option
   `coq-lsp.stats_on_hover_option` to re-enable it (@ejgallego, ejgallego/coq-lsp#412,
   fixes ejgallego/coq-lsp#398).
 - `coq-lsp` can now save `.vo` files for files opened in the
   editor. Use the new "Save to .vo" command, or the new protocol
   `coq/saveVo` request (@ejgallego, ejgallego/coq-lsp#417, fixes ejgallego/coq-lsp#339)
ejgallego added a commit to ejgallego/opam-repository that referenced this issue Feb 22, 2023
@ejgallego
[new release] coq-lsp (0.1.6+8.16)
53847ed 
CHANGES:

---------------------

 - The info / goal view now uses jsCoq's client-side rendering, with
   better highlighting and layout rendering (@artagnon, @ejgallego,
   ejgallego/coq-lsp#143, fixes ejgallego/coq-lsp#96)
 - Printing method is now configurable by the user (@ejgallego, ejgallego/coq-lsp#143,
   fixes ejgallego/coq-lsp#321)
 - Trigger completion on quote char "'" (@ejgallego, ejgallego/coq-lsp#350)
 - Fix typo on keybinding config for show goals (@tomtomjhj, ejgallego/coq-lsp#357)
 - New request `coq/getDocument` to get serialized full document
   contents. Thanks to Clément Pit-Claudel for feedback and ideas.
   (@ejgallego, ejgallego/coq-lsp#350)
 - Auto-ignore Coq object files; can be disabled in config
   (@ejgallego, ejgallego/coq-lsp#365)
 - Support workspaces with multiple roots, this is very useful for
   projects that contain several `_CoqProject` files in different
   directories (@ejgallego, ejgallego/coq-lsp#374)
 - Add VS Code commands to start / stop the server (@ejgallego, ejgallego/coq-lsp#377,
   cc ejgallego/coq-lsp#209)
 - Fix bug that made the server not exit on `exit` LSP notification
   (@artagnon, @ejgallego, ejgallego/coq-lsp#375, fixes ejgallego/coq-lsp#230)
 - Lay the foundation for server tests (@artagnon, ejgallego/coq-lsp#356)
 - Remove the `coq-lsp.ok_diagnostics` setting (@artagnon, ejgallego/coq-lsp#129)
 - Print abbreviations on hover (@ejgallego, ejgallego/coq-lsp#384)
 - Print hover types without parenthesis (@ejgallego, ejgallego/coq-lsp#384)
 - Parse identifiers with dot for hover and jump to definition
   (@ejallego, ejgallego/coq-lsp#385)
 - Update `vscode-languageclient` to 8.1.0 (@ejgallego, @Alizter,
   ejgallego/coq-lsp#383, fixes ejgallego/coq-lsp#273)
 - Fix typo on max_errors checking, this made coq-lsp stop on the
   number of total diagnostics, instead of only errors (@ejgallego,
   ejgallego/coq-lsp#386)
 - Hover symbol information: hypothesis names must shadow globals of
   the same name (@ejgallego, ejgallego/coq-lsp#391, fixes ejgallego/coq-lsp#388)
 - De-schedule document on didClose, otherwise the scheduler will keep
   trying to resume it if it didn't finish (@ejgallego, ejgallego/coq-lsp#392)
 - Hover symbol information: correctly handle identifiers before '.'
   and containing a quote (') themselves (@ejgallego, ejgallego/coq-lsp#393)
 - Add children entries to the table-of-contents (@ejgallego, ejgallego/coq-lsp#394)
 - Invalidate Coq's imperative cache on error (@ejgallego, @r-muhairi,
   ejgallego/coq-lsp#395)
 - Add status bar button to toggle server run status (@ejgallego,
   @Alizter, ejgallego/coq-lsp#378, closes ejgallego/coq-lsp#209)
 - Support for `COQLIB` and `COQCORELIB` environment variables, added
   `--coqcorelib` command line argument (@ejgallego, ejgallego/coq-lsp#403)
 - Protocol infrastructure for code lenses (@ejgallego, ejgallego/coq-lsp#396)
 - Set binary more for protocol input / output (@ejgallego, ejgallego/coq-lsp#408)
 - Allow to set `ocamlpath` from the command line (@ejgallego, ejgallego/coq-lsp#408)
 - Windows support (@ejgallego, @jim-portegies, ejgallego/coq-lsp#408)
 - Scroll active goal into view (@ejgallego, ejgallego/coq-lsp#410, fixes ejgallego/coq-lsp#381)
 - Server status icon will now react properly to fatal server errors
   (@ejgallego, reported by @Alizter, ejgallego/coq-lsp#411, fixes ejgallego/coq-lsp#399)
 - Info on memory and time is now disabled by default, new option
   `coq-lsp.stats_on_hover_option` to re-enable it (@ejgallego, ejgallego/coq-lsp#412,
   fixes ejgallego/coq-lsp#398).
 - `coq-lsp` can now save `.vo` files for files opened in the
   editor. Use the new "Save to .vo" command, or the new protocol
   `coq/saveVo` request (@ejgallego, ejgallego/coq-lsp#417, fixes ejgallego/coq-lsp#339)
@ejgallego ejgallego mentioned this issue Apr 3, 2023
Merged
ejgallego added a commit to ejgallego/opam-repository that referenced this issue Apr 3, 2023
@ejgallego
[new release] coq-lsp (0.1.6.1+8.17)
f1962a2 
CHANGES:

---------------------

 - The info / goal view now uses jsCoq's client-side rendering, with
   better highlighting and layout rendering (@artagnon, @ejgallego,
   ejgallego/coq-lsp#143, fixes ejgallego/coq-lsp#96)
 - Printing method is now configurable by the user (@ejgallego, ejgallego/coq-lsp#143,
   fixes ejgallego/coq-lsp#321)
 - Trigger completion on quote char "'" (@ejgallego, ejgallego/coq-lsp#350)
 - Fix typo on keybinding config for show goals (@tomtomjhj, ejgallego/coq-lsp#357)
 - New request `coq/getDocument` to get serialized full document
   contents. Thanks to Clément Pit-Claudel for feedback and ideas.
   (@ejgallego, ejgallego/coq-lsp#350)
 - Auto-ignore Coq object files; can be disabled in config
   (@ejgallego, ejgallego/coq-lsp#365)
 - Support workspaces with multiple roots, this is very useful for
   projects that contain several `_CoqProject` files in different
   directories (@ejgallego, ejgallego/coq-lsp#374)
 - Add VS Code commands to start / stop the server (@ejgallego, ejgallego/coq-lsp#377,
   cc ejgallego/coq-lsp#209)
 - Fix bug that made the server not exit on `exit` LSP notification
   (@artagnon, @ejgallego, ejgallego/coq-lsp#375, fixes ejgallego/coq-lsp#230)
 - Lay the foundation for server tests (@artagnon, ejgallego/coq-lsp#356)
 - Remove the `coq-lsp.ok_diagnostics` setting (@artagnon, ejgallego/coq-lsp#129)
 - Print abbreviations on hover (@ejgallego, ejgallego/coq-lsp#384)
 - Print hover types without parenthesis (@ejgallego, ejgallego/coq-lsp#384)
 - Parse identifiers with dot for hover and jump to definition
   (@ejallego, ejgallego/coq-lsp#385)
 - Update `vscode-languageclient` to 8.1.0 (@ejgallego, @Alizter,
   ejgallego/coq-lsp#383, fixes ejgallego/coq-lsp#273)
 - Fix typo on max_errors checking, this made coq-lsp stop on the
   number of total diagnostics, instead of only errors (@ejgallego,
   ejgallego/coq-lsp#386)
 - Hover symbol information: hypothesis names must shadow globals of
   the same name (@ejgallego, ejgallego/coq-lsp#391, fixes ejgallego/coq-lsp#388)
 - De-schedule document on didClose, otherwise the scheduler will keep
   trying to resume it if it didn't finish (@ejgallego, ejgallego/coq-lsp#392)
 - Hover symbol information: correctly handle identifiers before '.'
   and containing a quote (') themselves (@ejgallego, ejgallego/coq-lsp#393)
 - Add children entries to the table-of-contents (@ejgallego, ejgallego/coq-lsp#394)
 - Invalidate Coq's imperative cache on error (@ejgallego, @r-muhairi,
   ejgallego/coq-lsp#395)
 - Add status bar button to toggle server run status (@ejgallego,
   @Alizter, ejgallego/coq-lsp#378, closes ejgallego/coq-lsp#209)
 - Support for `COQLIB` and `COQCORELIB` environment variables, added
   `--coqcorelib` command line argument (@ejgallego, ejgallego/coq-lsp#403)
 - Protocol infrastructure for code lenses (@ejgallego, ejgallego/coq-lsp#396)
 - Set binary mode for protocol input / output (@ejgallego, ejgallego/coq-lsp#408)
 - Allow to set `ocamlpath` from the command line (@ejgallego, ejgallego/coq-lsp#408)
 - Windows support (@ejgallego, @jim-portegies, ejgallego/coq-lsp#408)
 - Scroll active goal into view (@ejgallego, ejgallego/coq-lsp#410, fixes ejgallego/coq-lsp#381)
 - Server status icon will now react properly to fatal server errors
   (@ejgallego, reported by @Alizter, ejgallego/coq-lsp#411, fixes ejgallego/coq-lsp#399)
 - Info on memory and time is now disabled by default, new option
   `coq-lsp.stats_on_hover_option` to re-enable it (@ejgallego, ejgallego/coq-lsp#412,
   fixes ejgallego/coq-lsp#398).
 - `coq-lsp` can now save `.vo` files for files opened in the
   editor. Use the new "Save to .vo" command, or the new protocol
   `coq/saveVo` request (@ejgallego, ejgallego/coq-lsp#417, fixes ejgallego/coq-lsp#339)
ejgallego added a commit to ejgallego/opam-repository that referenced this issue Apr 3, 2023
@ejgallego
[new release] coq-lsp (0.1.6.1+8.17)
3c26d05 
CHANGES:

---------------------

 - The info / goal view now uses jsCoq's client-side rendering, with
   better highlighting and layout rendering (@artagnon, @ejgallego,
   ejgallego/coq-lsp#143, fixes ejgallego/coq-lsp#96)
 - Printing method is now configurable by the user (@ejgallego, ejgallego/coq-lsp#143,
   fixes ejgallego/coq-lsp#321)
 - Trigger completion on quote char "'" (@ejgallego, ejgallego/coq-lsp#350)
 - Fix typo on keybinding config for show goals (@tomtomjhj, ejgallego/coq-lsp#357)
 - New request `coq/getDocument` to get serialized full document
   contents. Thanks to Clément Pit-Claudel for feedback and ideas.
   (@ejgallego, ejgallego/coq-lsp#350)
 - Auto-ignore Coq object files; can be disabled in config
   (@ejgallego, ejgallego/coq-lsp#365)
 - Support workspaces with multiple roots, this is very useful for
   projects that contain several `_CoqProject` files in different
   directories (@ejgallego, ejgallego/coq-lsp#374)
 - Add VS Code commands to start / stop the server (@ejgallego, ejgallego/coq-lsp#377,
   cc ejgallego/coq-lsp#209)
 - Fix bug that made the server not exit on `exit` LSP notification
   (@artagnon, @ejgallego, ejgallego/coq-lsp#375, fixes ejgallego/coq-lsp#230)
 - Lay the foundation for server tests (@artagnon, ejgallego/coq-lsp#356)
 - Remove the `coq-lsp.ok_diagnostics` setting (@artagnon, ejgallego/coq-lsp#129)
 - Print abbreviations on hover (@ejgallego, ejgallego/coq-lsp#384)
 - Print hover types without parenthesis (@ejgallego, ejgallego/coq-lsp#384)
 - Parse identifiers with dot for hover and jump to definition
   (@ejallego, ejgallego/coq-lsp#385)
 - Update `vscode-languageclient` to 8.1.0 (@ejgallego, @Alizter,
   ejgallego/coq-lsp#383, fixes ejgallego/coq-lsp#273)
 - Fix typo on max_errors checking, this made coq-lsp stop on the
   number of total diagnostics, instead of only errors (@ejgallego,
   ejgallego/coq-lsp#386)
 - Hover symbol information: hypothesis names must shadow globals of
   the same name (@ejgallego, ejgallego/coq-lsp#391, fixes ejgallego/coq-lsp#388)
 - De-schedule document on didClose, otherwise the scheduler will keep
   trying to resume it if it didn't finish (@ejgallego, ejgallego/coq-lsp#392)
 - Hover symbol information: correctly handle identifiers before '.'
   and containing a quote (') themselves (@ejgallego, ejgallego/coq-lsp#393)
 - Add children entries to the table-of-contents (@ejgallego, ejgallego/coq-lsp#394)
 - Invalidate Coq's imperative cache on error (@ejgallego, @r-muhairi,
   ejgallego/coq-lsp#395)
 - Add status bar button to toggle server run status (@ejgallego,
   @Alizter, ejgallego/coq-lsp#378, closes ejgallego/coq-lsp#209)
 - Support for `COQLIB` and `COQCORELIB` environment variables, added
   `--coqcorelib` command line argument (@ejgallego, ejgallego/coq-lsp#403)
 - Protocol infrastructure for code lenses (@ejgallego, ejgallego/coq-lsp#396)
 - Set binary mode for protocol input / output (@ejgallego, ejgallego/coq-lsp#408)
 - Allow to set `ocamlpath` from the command line (@ejgallego, ejgallego/coq-lsp#408)
 - Windows support (@ejgallego, @jim-portegies, ejgallego/coq-lsp#408)
 - Scroll active goal into view (@ejgallego, ejgallego/coq-lsp#410, fixes ejgallego/coq-lsp#381)
 - Server status icon will now react properly to fatal server errors
   (@ejgallego, reported by @Alizter, ejgallego/coq-lsp#411, fixes ejgallego/coq-lsp#399)
 - Info on memory and time is now disabled by default, new option
   `coq-lsp.stats_on_hover_option` to re-enable it (@ejgallego, ejgallego/coq-lsp#412,
   fixes ejgallego/coq-lsp#398).
 - `coq-lsp` can now save `.vo` files for files opened in the
   editor. Use the new "Save to .vo" command, or the new protocol
   `coq/saveVo` request (@ejgallego, ejgallego/coq-lsp#417, fixes ejgallego/coq-lsp#339)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind: bug Something isn't working needs: reproduction method
Projects
coq-lsp roadmap
Archived in project
Milestone
0.1.6
2 participants
@ejgallego @Alizter