Skip to content

Ensure exit notification is sent before closing connection #776

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

Merged
merged 3 commits into from
Jun 23, 2021

Conversation

dsherret
Copy link
Contributor

@dsherret dsherret commented Jun 19, 2021

Currently the exit notification is not sent because the connection gets closed before the message is sent (at least on my machine in WSL). This change waits for the exit notification to be sent before closing the connection.

Fixes #726 (exit notification not received part).

@dsherret dsherret marked this pull request as ready for review June 19, 2021 01:18
@dbaeumer
Copy link
Member

Good catch. Thanks for the PR.

@dbaeumer dbaeumer merged commit 6eef6d2 into microsoft:main Jun 23, 2021
@dsherret dsherret deleted the fix-exit-notification branch June 23, 2021 14:35
@bknaysi
Copy link

bknaysi commented Jan 4, 2022

Hello @dbaeumer and @dsherret, the fix doesn't appear to be in the 7.0.0 client release or 7.1.0.next.5 tag. When do you think there will be a client release at version 7.0.0 or higher containing this fix?

@dbaeumer
Copy link
Member

dbaeumer commented Jan 6, 2022

The fix is in the 8.x-next.y release of the library

@bknaysi
Copy link

bknaysi commented Jan 6, 2022

@dbaeumer Thanks for the reply. It looks like 8.x-next.y requires the 3.17.0 LSP Protocol. However, there doesn't exist an LSP4J release supporting it yet, which we use to implement our server. Would it be possible for our client to use 8.x-next.y while our server only supports the 3.16.0 LSP Protocol? I'm not sure which direction the backwards compatibility exist. For example, could a client on an older protocol work with a server on a newer protocol? Likewise, could a server on an older protocol work with a client on a newer protocol?

If the later is not true, it would be amazing if this exit notification fix could be added to the future 7.1.0 release. I'm not sure we'll want to adopt any transitory next releases, but we can wait until another official one.

@dbaeumer
Copy link
Member

dbaeumer commented Jan 7, 2022

I think the 8.x-next.y client should be able to talk to a 3.16 server. The protocol is compatible if the server correctly handles capability flags.

@bknaysi
Copy link

bknaysi commented Jan 7, 2022

I think we'll wait for the official 8.0.0 release before adopting it into our product, but it's good to know they should be compatible. In general, how stable are the intermittent -next releases?

@dbaeumer
Copy link
Member

I usually adopt them in ESLint. The proposed feature might still change. All the other stuff is fairly stable.

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 this pull request may close these issues.

Orphaned server processes when VS Code client process is killed
3 participants