java server refreshing the workspace for each restart on Windows #880

snjeza · 2019-04-12T03:51:31Z

Fixes #793

Signed-off-by: Snjezana Peco snjezana.peco@redhat.com

fbricon · 2019-04-12T19:25:10Z

src/javaServerStarter.ts

@@ -66,12 +67,6 @@ function prepareParams(requirements: RequirementsData, javaConfiguration, worksp
 	if (vmargs.indexOf(encodingKey) < 0) {
 		params.push(encodingKey + getJavaEncoding());
 	}
-	if (os.platform() == 'win32') {


why is this removed?

VS Code doesn't kill Java LS server anymore. See microsoft/vscode#35196 and https://github.com/Microsoft/vscode-languageserver-node/blob/master/client/src/main.ts#L418

fbricon · 2019-04-12T19:36:29Z

Restarting spring-boot workspace, I see the shutdown doesn't complete, and the workspace is still not properly saved:

!ENTRY org.eclipse.jdt.ls.core 1 0 2019-04-12 15:32:09.126
!MESSAGE >> shutdown

!ENTRY org.eclipse.jdt.ls.core 1 0 2019-04-12 15:32:09.127
!MESSAGE Saving workspace
!SESSION 2019-04-12 15:32:14.400 -----------------------------------------------
eclipse.buildId=unknown
java.version=1.8.0_181
java.vendor=Oracle Corporation
BootLoader constants: OS=macosx, ARCH=x86_64, WS=cocoa, NL=en_CA
Command-line arguments:  -data /Users/fbricon/Library/Application Support/Code - Insiders/User/workspaceStorage/4b108579b42a00cd8343838a6a610090/redhat.java/jdt_ws

!ENTRY org.eclipse.core.resources 2 10035 2019-04-12 15:32:15.906
!MESSAGE The workspace exited with unsaved changes in the previous session; refreshing workspace to recover changes.

snjeza · 2019-04-12T20:02:12Z

Restarting spring-boot workspace, I see the shutdown doesn't complete, and the workspace is still not properly saved:

Could you test with https://raw.githubusercontent.com/snjeza/vscode-test/master/java-0.43.1.vsix ?

fbricon · 2019-04-12T20:27:55Z

This is on another workspace, but this looks much better, shutdown takes several seconds to complete, but does complete with your special build:


!ENTRY org.eclipse.jdt.ls.core 1 0 2019-04-12 16:25:34.321
!MESSAGE >> shutdown

!ENTRY org.eclipse.jdt.ls.core 1 0 2019-04-12 16:25:34.321
!MESSAGE Saving workspace

!ENTRY org.eclipse.jdt.ls.core 1 0 2019-04-12 16:25:37.599
!MESSAGE Workspace saved. Took 3277ms

!ENTRY org.eclipse.jdt.ls.core 1 0 2019-04-12 16:25:37.600
!MESSAGE Shutdown received... waking up main thread

!ENTRY org.eclipse.jdt.ls.core 1 0 2019-04-12 16:25:37.605
!MESSAGE class org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin is stopping:
!SESSION 2019-04-12 16:25:39.447 -----------------------------------------------
eclipse.buildId=unknown
java.version=1.8.0_181
java.vendor=Oracle Corporation
BootLoader constants: OS=macosx, ARCH=x86_64, WS=cocoa, NL=en_CA
Command-line arguments:  -data /Users/fbricon/Library/Application Support/Code/User/workspaceStorage/4b108579b42a00cd8343838a6a610090/redhat.java/jdt_ws

!ENTRY org.eclipse.jdt.ls.core 1 0 2019-04-12 16:25:45.133
!MESSAGE class org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin is started

!ENTRY org.eclipse.jdt.ls.core 1 0 2019-04-12 16:25:45.261
!MESSAGE Main thread is waiting

fbricon · 2019-04-25T19:08:57Z

src/javaServerStarter.ts

+	if (os.platform() === 'win32') {
+		const vmargs = javaConfig.get('jdt.ls.vmargs', '');
+		const watchParentProcess = '-DwatchParentProcess=';
+		if (vmargs.indexOf(watchParentProcess) >= 0) {


watchParentProcess is always present, as per L78

This code is called before the prepareParams method (L78).

@snjeza , why do we need to set detatched to true only on Windows ? Does https://github.com/microsoft/vscode-languageserver-node/blob/e7b7e3134791c4208165aa57fb9182895df8ec6c/client/src/node/main.ts#L201 just work on Linux/MacOS ?

Yes, it just works. I think it is the default on Linux/Mac.

yaohaizh · 2019-04-26T06:42:23Z

@fbricon @snjeza Could this change microsoft/vscode-languageserver-node#485 fix the parent watching process issue?

snjeza · 2021-12-04T16:44:09Z

I have updated the PR.

jdneo · 2022-09-28T06:49:13Z

@snjeza I tested this change on Windows and observed that after reloading the window, the message !MESSAGE The workspace exited with unsaved changes in the previous session; refreshing workspace to recover changes. will not be seen anymore.

One question: I think the key point is options.detached = true;, but according to current change, it will only be set when vmargs contains -DwatchParentProcess=, which is not contained in the default value of the vmargs. What's the purpose for that check?

snjeza · 2022-09-28T14:06:00Z

One question: I think the key point is options.detached = true;, but according to current change, it will only be set when vmargs contains -DwatchParentProcess=, which is not contained in the default value of the vmargs. What's the purpose for that check?

We could set it as the default. It will not run tasklist (on Windows) or kill (on Linux/Mac)t after eclipse-jdtls/eclipse.jdt.ls#2115
This way, Java LS will be shut down on Windows properly.
@fbricon @rgrunber @testforstephen @CsCherrYY

rgrunber · 2022-09-28T18:06:31Z

@jdneo @snjeza How does this change solve the issue on Windows, with only defaults if -DwatchParentProcess=false is never passed as a JVM argument. It only gets set to false in prepareParams, which happens after the change in this PR. Did you manually set -DwatchParentProcess=false in java.jdt.ls.vmargs ?

snjeza · 2022-09-28T18:33:04Z

@rgrunber you should set -DwatchParentProcess=30, ~~30 seconds delay~~

rgrunber · 2022-09-28T21:05:13Z

If this helps the situation, then I'm fine with merging. I just don't see how it would benefit the issue without users also overriding the default java.jdt.ls.vmargs in their settings. But if a manual change to a setting is required then most users likely won't benefit.

snjeza · 2022-09-28T21:29:41Z

If this helps the situation, then I'm fine with merging. I just don't see how it would benefit the issue without users also overriding the default java.jdt.ls.vmargs in their settings. But if a manual change to a setting is required then most users likely won't benefit.

See #880 (comment)

jdneo · 2022-09-29T00:47:52Z

When I tested this change, I manually changed the vmargs. options.detached = true; seems to work. I'm also confused the check for the -DwatchParentProcess.

@snjeza Could you help me understand the relationship between -DwatchParentProcess and options.detached = true;?

snjeza · 2022-09-29T01:39:18Z

Could you help me understand the relationship between -DwatchParentProcess and options.detached = true;?

We have disabled watchParentProcess on Windows because Windows kills child processes when the parent process finishes.
This way, Java LS will not save a big workspace properly.
options.detached=true requires that Windows doesn't kill child processes and Java LS saves the workspace properly.
I have updated the PR. Now, it works in the following way:

VS Code sets options.detached=true, watchParentProcess is not set, Java LS will exit and save the workspace properly when its parent process is dead. We can do it after Use ProcessHandle.of(long) to determine state of the parent process. eclipse-jdtls/eclipse.jdt.ls#2115 because Java LS will not start the tasklist command anymore.
you can add -DwatchParentProcess=false to vmargs if you want old behaviour

Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>

jdneo

I tested on the Windows machine.

Opening eclipse.jdt.ls project:

Without the change, takes around 60s
With the change, it takes around 30s.

testforstephen · 2022-09-29T09:46:54Z

Just found the previous background eclipse/lemminx#328 to disable parentProcessWatcher by default. Since we've optimized the server-side parentProcessWatcher via the JDK native API, it is worth to enable it back if the performance gains are high. I suggest we put it in pre-release for a try first.

snjeza requested review from gorkem, fbricon and yaohaizh April 12, 2019 03:51

snjeza mentioned this pull request Apr 12, 2019

Fix build on Windows eclipse-jdtls/eclipse.jdt.ls#991

Closed

yaohaizh approved these changes Apr 12, 2019

View reviewed changes

fbricon reviewed Apr 12, 2019

View reviewed changes

snjeza changed the title ~~Fix build on Windows~~ [WIP] Fix build on Windows Apr 12, 2019

snjeza changed the title ~~[WIP] Fix build on Windows~~ Fix build on Windows Apr 12, 2019

snjeza changed the title ~~Fix build on Windows~~ {WIP] Fix build on Windows Apr 21, 2019

snjeza force-pushed the issue-793 branch from 73b5a08 to 2dc9d8b Compare April 23, 2019 10:02

snjeza changed the title ~~{WIP] Fix build on Windows~~ Fix build on Windows Apr 23, 2019

fbricon reviewed Apr 25, 2019

View reviewed changes

snjeza force-pushed the issue-793 branch from 2dc9d8b to d371309 Compare May 2, 2019 21:09

snjeza mentioned this pull request Nov 14, 2019

Java debugging broken on maven project on update to VScode 1.4.0 microsoft/vscode-java-debug#713

Closed

github-actions bot added the has conflicts label Jul 16, 2020

snjeza mentioned this pull request Nov 24, 2021

java server refreshing the workspace (cleaning and building) for each restart eclipse-jdtls/eclipse.jdt.ls#1948

Merged

snjeza changed the title ~~Fix build on Windows~~ [WIP] Fix build on Windows Nov 24, 2021

openshift-ci bot added the do-not-merge/work-in-progress label Nov 24, 2021

snjeza added the in progress label Nov 24, 2021

snjeza force-pushed the issue-793 branch from d371309 to f0103c9 Compare December 4, 2021 16:39

snjeza changed the title ~~[WIP] Fix build on Windows~~ java server refreshing the workspace for each restart on Windows Dec 4, 2021

openshift-ci bot removed the do-not-merge/work-in-progress label Dec 4, 2021

snjeza requested review from rgrunber and testforstephen December 4, 2021 16:41

snjeza removed the in progress label Dec 4, 2021

snjeza removed the has conflicts label Dec 4, 2021

github-actions bot added the has conflicts label Dec 16, 2021

snyk-bot mentioned this pull request Apr 1, 2022

[Snyk] Upgrade fs-extra from 8.1.0 to 10.0.1 Manny27nyc/vscode-java#6

Open

snjeza mentioned this pull request Sep 27, 2022

Project cannot be synced when gradle init script path contains white spaces #2692

Closed

snjeza force-pushed the issue-793 branch from f0103c9 to 1dd00fb Compare September 27, 2022 21:53

github-actions bot removed the has conflicts label Sep 28, 2022

snjeza force-pushed the issue-793 branch from 1dd00fb to 47ea1b5 Compare September 29, 2022 01:28

java server refreshing the workspace for each restart on Windows

f2b1302

Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>

snjeza force-pushed the issue-793 branch from 47ea1b5 to f2b1302 Compare September 29, 2022 01:48

jdneo approved these changes Sep 29, 2022

View reviewed changes

snjeza merged commit 5643a2e into redhat-developer:master Sep 29, 2022

jdneo mentioned this pull request Oct 8, 2022

Improve project initialization eclipse-jdtls/eclipse.jdt.ls#2252

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

java server refreshing the workspace for each restart on Windows #880

java server refreshing the workspace for each restart on Windows #880

snjeza commented Apr 12, 2019 •

edited

Loading

fbricon Apr 12, 2019

snjeza Apr 12, 2019

fbricon commented Apr 12, 2019

snjeza commented Apr 12, 2019 •

edited

Loading

fbricon commented Apr 12, 2019 •

edited

Loading

fbricon Apr 25, 2019

snjeza Apr 25, 2019

rgrunber Sep 27, 2022 •

edited

Loading

snjeza Sep 27, 2022

yaohaizh commented Apr 26, 2019

snjeza commented Dec 4, 2021

jdneo commented Sep 28, 2022

snjeza commented Sep 28, 2022

rgrunber commented Sep 28, 2022 •

edited

Loading

snjeza commented Sep 28, 2022 •

edited

Loading

rgrunber commented Sep 28, 2022

snjeza commented Sep 28, 2022

jdneo commented Sep 29, 2022

snjeza commented Sep 29, 2022 •

edited

Loading

jdneo left a comment

testforstephen commented Sep 29, 2022 •

edited

Loading

java server refreshing the workspace for each restart on Windows #880

java server refreshing the workspace for each restart on Windows #880

Conversation

snjeza commented Apr 12, 2019 • edited Loading

fbricon Apr 12, 2019

Choose a reason for hiding this comment

snjeza Apr 12, 2019

Choose a reason for hiding this comment

fbricon commented Apr 12, 2019

snjeza commented Apr 12, 2019 • edited Loading

fbricon commented Apr 12, 2019 • edited Loading

fbricon Apr 25, 2019

Choose a reason for hiding this comment

snjeza Apr 25, 2019

Choose a reason for hiding this comment

rgrunber Sep 27, 2022 • edited Loading

Choose a reason for hiding this comment

snjeza Sep 27, 2022

Choose a reason for hiding this comment

yaohaizh commented Apr 26, 2019

snjeza commented Dec 4, 2021

jdneo commented Sep 28, 2022

snjeza commented Sep 28, 2022

rgrunber commented Sep 28, 2022 • edited Loading

snjeza commented Sep 28, 2022 • edited Loading

rgrunber commented Sep 28, 2022

snjeza commented Sep 28, 2022

jdneo commented Sep 29, 2022

snjeza commented Sep 29, 2022 • edited Loading

jdneo left a comment

Choose a reason for hiding this comment

testforstephen commented Sep 29, 2022 • edited Loading

snjeza commented Apr 12, 2019 •

edited

Loading

snjeza commented Apr 12, 2019 •

edited

Loading

fbricon commented Apr 12, 2019 •

edited

Loading

rgrunber Sep 27, 2022 •

edited

Loading

rgrunber commented Sep 28, 2022 •

edited

Loading

snjeza commented Sep 28, 2022 •

edited

Loading

snjeza commented Sep 29, 2022 •

edited

Loading

testforstephen commented Sep 29, 2022 •

edited

Loading