You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2020/11/05 15:03:29 UTC
[GitHub] [netbeans] sdedic opened a new pull request #2523: Prevent race conditions during CLI install
sdedic opened a new pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523
There are a lot of issues when calling `--modules` from the CLI client. Full / robus fix is IMHO not possible for 12.2 timeframe: there's a race condition between `LifecycleManager.exit()` call and the CLI server's return of the exit status - so the CLI client may not be even able to realize it's just a client connecting to a server - it fails with E_CANNOT_CONNECT.
This should be fixed for next NB release anyway; CLI connecting to a running process should never interfere with the installation files.
The vscode extension now fully stops the language server, will run another NB process just for installation. That process will create a JVM and will do the update process after JVM terminates; in the 2nd run, it finds `netbeans.close` property and will terminate.
Then the real Language Server will be restarted.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] JaroslavTulach commented on a change in pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
JaroslavTulach commented on a change in pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#discussion_r519143219
##########
File path: java/java.lsp.server/vscode/src/extension.ts
##########
@@ -346,14 +388,26 @@ function activateWithJDK(specifiedJDK: string | null, context: ExtensionContext,
const yes = "Install GPLv2+CPEx code";
window.showErrorMessage("Additional Java Support is needed", yes).then(reply => {
if (yes === reply) {
- let installProcess = launcher.launch(info, "--modules", "--install", ".*nbjavac.*");
+ vscode.window.setStatusBarMessage("Preparing Apache NetBeans Language Server for additional installation", 2000);
+ restartWithJDKLater = function() {
+ log.appendLine("Ignoring request for restart of Apache NetBeans Language Server");
+ };
+ killNbProcess(false);
+ let installProcess = launcher.launch(info, "-J-Dnetbeans.close=true", "--modules", "--install", ".*nbjavac.*");
let logData = function(d: any) {
log.append(d.toString());
};
installProcess.stdout.on('data', logData);
installProcess.stderr.on('data', logData);
installProcess.on('close', function(code: number) {
log.append("Additional Java Support installed with exit code " + code);
+ // let the updater to settle down.
+ activateWithJDK(specifiedJDK, context, log, notifyKill);
Review comment:
...let the installer start new copy of Apache NetBeans Language Server explicitly.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] JaroslavTulach commented on a change in pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
JaroslavTulach commented on a change in pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#discussion_r519142331
##########
File path: java/java.lsp.server/vscode/src/extension.ts
##########
@@ -163,14 +163,45 @@ export function activate(context: ExtensionContext) {
}));
}
+/**
+ * Pending maintenance (install) task, activations should be chained after it.
+ */
+let maintenance : Promise<void> | null;
+
+/**
+ * Pending activation flag. Will be cleared when the process produces some message or fails.
+ */
+let activationPending : boolean = false;
+
function activateWithJDK(specifiedJDK: string | null, context: ExtensionContext, log : vscode.OutputChannel, notifyKill: boolean): void {
- if (nbProcess) {
+ const a : Promise<void> | null = maintenance;
+ if (activationPending) {
+ // do not activate more than once in parallel.
+ return;
+ }
+ activationPending = true;
+ if (a != null) {
+ a.then(() => doActivateWithJDK(specifiedJDK, context, log, notifyKill));
+ } else {
+ doActivateWithJDK(specifiedJDK, context, log, notifyKill);
+ }
+}
+
+function killNbProcess(notifyKill : boolean, specProcess?: ChildProcess) {
+ let p = nbProcess;
+ if (p && (!specProcess || specProcess == p)) {
if (notifyKill) {
vscode.window.setStatusBarMessage("Restarting Apache NetBeans Language Server.", 2000);
}
- nbProcess.kill();
+ p.kill();
+ // PENDING: possible race
Review comment:
Everything in one extension runs in a single process.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] JaroslavTulach commented on pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
JaroslavTulach commented on pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#issuecomment-723485811
I tested latest Sváťa's change on Win8 and it seems to work. It would be great, if we can get it in for 12.2, @lkishalmi.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] JaroslavTulach commented on pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
JaroslavTulach commented on pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#issuecomment-723402317
Sváťo, I have just added 315d120 with the hope to prevent more races... if my commit isn't sane, feel free to `get reset HEAD~1` - e.g. remove it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] JaroslavTulach commented on a change in pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
JaroslavTulach commented on a change in pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#discussion_r519259095
##########
File path: platform/o.n.bootstrap/launcher/windows/platformlauncher.cpp
##########
@@ -714,7 +720,8 @@ void PlatformLauncher::onExit() {
STARTUPINFO si = {0};
PROCESS_INFORMATION pi = {0};
si.cb = sizeof(STARTUPINFO);
- if (!CreateProcess(NULL, cmdLineStr, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi)) {
+
+ if (!CreateProcess(NULL, cmdLineStr, NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi)) {
Review comment:
The Win32 API for [CreateProcess](https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessa) says:
_bInheritHandles: If this parameter is TRUE, each inheritable handle in the calling process is inherited by the new process.... Note that inherited handles have the same value and access rights as the original handles..._
By passing `TRUE` we request typical UNIX-like behavior.
Then there is a note: _Terminal Services: You cannot inherit handles across sessions. Additionally, if this parameter is TRUE, you must create the process in the same session as the caller._ where _session_ probably means [Remote Desktop Services session under which the specified process is running](https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-processidtosessionid) which is certainly not something we mangle. E.g. we should be safe to pass `TRUE` in our calls.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] sdedic edited a comment on pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
sdedic edited a comment on pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#issuecomment-723549281
@jtulach @lkishalmi I'm really sorry for the number; I wanted to bump it up, but I simply mistyped the release number and the error passed self-review before commit.
Looking at the version scheme, we seem to had:
* launcher 9.0 at the time of 10.0 release (https://github.com/apache/netbeans/commit/7cac305080542ae7f68f546a5a63a7f9c06bbb40)
* launcher 10.0 at the time of 11.0 release (https://github.com/apache/netbeans/commit/fc9a290b3d7a90cba4af3d04d9573fefff0a5939)
before that, the launcher version was synced with the release version (8.2)
The current fix is not a functional change; so we can either have 10.1 (switches same as 10.0) or 12.2; I couldn't find 12.1 binary:
```
3289B87AB9345958E16F3285ED884F5C4DAB7C2D-platform-launchers-10.0.zip
435688D7DE27C5EE5FAF666D4C6E63DE949ADF0B-platform-launchers-8.2.zip
452B8E1C0922AADEC9D5B63BA8AAEC40FBD2DBAB-platform-launchers-9.0.zip
7A91F97F315B9176838FD15E16C0B9AEC9E6A226-platform-launchers-10.0.zip
A67A12FFB2966457F5FAAE2204720786343C83CC-platform-launchers-9.0.zip
D8A083D00A073359E87B5A077D98541B12E31304-platform-launchers-9.0.zip
```
My personal preference is 12.2 (= release).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] sdedic commented on pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
sdedic commented on pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#issuecomment-723487731
fixup: unchanged version in the license metadata header
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] sdedic commented on pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
sdedic commented on pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#issuecomment-723420886
Additional complication on Windows: the nbexec launcher re-launches itself from onExit(), but the original process represented by `ChildProcess` in JS code exits. So we execute the launcher to perform update - launch (and terminate, because of `netbeans.close`) sequence. But the ChildProcess exits after the 1st step, leaving nbexec for the `launch` to run. So listening for ChildProcess.on('exit') is clearly wrong (on Windows).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] lkishalmi commented on a change in pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
lkishalmi commented on a change in pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#discussion_r519271921
##########
File path: platform/o.n.bootstrap/external/platform-launchers-12.3-license.txt
##########
@@ -1,6 +1,6 @@
Name: NetBeans Application Launchers
Description: Windows Launchers for the NetBeans Platform
-Version: 10.0
+Version: 12.3
Review comment:
Well, the launcher zip is only used when not building that from sources which is true almost always when not building a release. So I could understand that could be used for testing, still the last released launcher version is 12.1.
I'm going to go to sleep now, checking this PR in about 8 hours from now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] JaroslavTulach commented on a change in pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
JaroslavTulach commented on a change in pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#discussion_r519142770
##########
File path: java/java.lsp.server/vscode/src/extension.ts
##########
@@ -288,7 +328,8 @@ function activateWithJDK(specifiedJDK: string | null, context: ExtensionContext,
return ErrorAction.Continue;
},
closed : function(): CloseAction {
- activateWithJDK(specifiedJDK, context, log, false);
+ log.appendLine("Connection to Apache NetBeans Language Server closed.");
+ restartWithJDKLater(10000, false);
Review comment:
Let's play safe for 12.2: we don't have to restart immediately. Let's put there a timeout. Moreover, let's allow disabling this restart altogether...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] JaroslavTulach commented on a change in pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
JaroslavTulach commented on a change in pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#discussion_r518262524
##########
File path: platform/o.n.bootstrap/launcher/windows/nbexec.cpp
##########
@@ -43,9 +43,16 @@ extern "C" BOOL APIENTRY DllMain(HANDLE hModule,
return TRUE;
}
+volatile int exitStatus = 0;
+
void exitHook(int status) {
+ exitStatus = status;
logMsg("Exit hook called with status %d", status);
- launcher.onExit();
+ // do not handle possible restarts, if we are just CLI-connecting to a running process.
+ if (status != -252) {
Review comment:
OK.
##########
File path: java/java.lsp.server/vscode/src/extension.ts
##########
@@ -163,14 +163,45 @@ export function activate(context: ExtensionContext) {
}));
}
+/**
+ * Pending maintenance (install) task, activations should be chained after it.
+ */
+let maintenance : Promise<void> | null;
+
+/**
+ * Pending activation flag. Will be cleared when the process produces some message or fails.
+ */
+let activationPending : boolean = false;
Review comment:
I would prefer to reduce this static state somehow. Not sure how yet.
##########
File path: platform/o.n.bootstrap/src/org/netbeans/MainImpl.java
##########
@@ -60,7 +60,7 @@ public static void main (String args[]) throws Exception {
int res = execute (args, System.in, System.out, System.err, m);
if (res == -1) {
// Connected to another running NB instance and succeeded in making a call.
- return;
+ System.exit(CLIHandler.Status.CONNECTED);
Review comment:
OK.
##########
File path: platform/o.n.bootstrap/launcher/windows/platformlauncher.cpp
##########
@@ -662,6 +664,10 @@ bool PlatformLauncher::restartRequested() {
void PlatformLauncher::onExit() {
logMsg("onExit()");
+ if (exitStatus == -252) {
Review comment:
OK.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] JaroslavTulach commented on a change in pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
JaroslavTulach commented on a change in pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#discussion_r518777355
##########
File path: java/java.lsp.server/vscode/src/extension.ts
##########
@@ -163,14 +163,45 @@ export function activate(context: ExtensionContext) {
}));
}
+/**
+ * Pending maintenance (install) task, activations should be chained after it.
+ */
+let maintenance : Promise<void> | null;
+
+/**
+ * Pending activation flag. Will be cleared when the process produces some message or fails.
+ */
+let activationPending : boolean = false;
+
function activateWithJDK(specifiedJDK: string | null, context: ExtensionContext, log : vscode.OutputChannel, notifyKill: boolean): void {
- if (nbProcess) {
+ const a : Promise<void> | null = maintenance;
+ if (activationPending) {
+ // do not activate more than once in parallel.
+ return;
+ }
+ activationPending = true;
+ if (a != null) {
+ a.then(() => doActivateWithJDK(specifiedJDK, context, log, notifyKill));
+ } else {
+ doActivateWithJDK(specifiedJDK, context, log, notifyKill);
+ }
+}
+
+function killNbProcess(notifyKill : boolean, specProcess?: ChildProcess) {
+ let p = nbProcess;
+ if (p && (!specProcess || specProcess == p)) {
if (notifyKill) {
vscode.window.setStatusBarMessage("Restarting Apache NetBeans Language Server.", 2000);
}
- nbProcess.kill();
+ p.kill();
+ // PENDING: possible race
Review comment:
What do you mean by possible race, @sdedic?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] JaroslavTulach commented on a change in pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
JaroslavTulach commented on a change in pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#discussion_r518673985
##########
File path: platform/o.n.bootstrap/launcher/windows/platformlauncher.cpp
##########
@@ -662,6 +664,10 @@ bool PlatformLauncher::restartRequested() {
void PlatformLauncher::onExit() {
logMsg("onExit()");
+ if (exitStatus == -252) {
Review comment:
How does it work with `nbexec*.exe` or `nbexec*.dll`? Do the binaries also need to be uploaded somewhere?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] JaroslavTulach commented on pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
JaroslavTulach commented on pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#issuecomment-723536836
> I've tested that on Windows 10, nbjavac can be uninstalled from NB instance where it was active/running (so it had locked its JARs) without any issues.
I've tested that as well. It seems to work.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] JaroslavTulach commented on a change in pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
JaroslavTulach commented on a change in pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#discussion_r519143040
##########
File path: java/java.lsp.server/vscode/src/extension.ts
##########
@@ -346,14 +388,26 @@ function activateWithJDK(specifiedJDK: string | null, context: ExtensionContext,
const yes = "Install GPLv2+CPEx code";
window.showErrorMessage("Additional Java Support is needed", yes).then(reply => {
if (yes === reply) {
- let installProcess = launcher.launch(info, "--modules", "--install", ".*nbjavac.*");
+ vscode.window.setStatusBarMessage("Preparing Apache NetBeans Language Server for additional installation", 2000);
+ restartWithJDKLater = function() {
Review comment:
...before installing, let's replace the `restartWithJDKLater` function with a dummy wrapper that does nothing and...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] sdedic commented on a change in pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
sdedic commented on a change in pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#discussion_r518820876
##########
File path: java/java.lsp.server/vscode/src/extension.ts
##########
@@ -163,14 +163,45 @@ export function activate(context: ExtensionContext) {
}));
}
+/**
+ * Pending maintenance (install) task, activations should be chained after it.
+ */
+let maintenance : Promise<void> | null;
+
+/**
+ * Pending activation flag. Will be cleared when the process produces some message or fails.
+ */
+let activationPending : boolean = false;
+
function activateWithJDK(specifiedJDK: string | null, context: ExtensionContext, log : vscode.OutputChannel, notifyKill: boolean): void {
- if (nbProcess) {
+ const a : Promise<void> | null = maintenance;
+ if (activationPending) {
+ // do not activate more than once in parallel.
+ return;
+ }
+ activationPending = true;
+ if (a != null) {
+ a.then(() => doActivateWithJDK(specifiedJDK, context, log, notifyKill));
+ } else {
+ doActivateWithJDK(specifiedJDK, context, log, notifyKill);
+ }
+}
+
+function killNbProcess(notifyKill : boolean, specProcess?: ChildProcess) {
+ let p = nbProcess;
+ if (p && (!specProcess || specProcess == p)) {
if (notifyKill) {
vscode.window.setStatusBarMessage("Restarting Apache NetBeans Language Server.", 2000);
}
- nbProcess.kill();
+ p.kill();
+ // PENDING: possible race
Review comment:
leftover; Promise reordering no longer happens, and the nodejs runtime schedules all callbacks in a single thread, right ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] lkishalmi commented on a change in pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
lkishalmi commented on a change in pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#discussion_r519250870
##########
File path: platform/o.n.bootstrap/external/platform-launchers-12.3-license.txt
##########
@@ -1,6 +1,6 @@
Name: NetBeans Application Launchers
Description: Windows Launchers for the NetBeans Platform
-Version: 10.0
+Version: 12.3
Review comment:
Well, is it really necessary to implement time travel here as we are just about to release NetBeans 12.2. Where the number 12.3 is coming from? So is this intentional or mistype or some coincidence or something has happend that I'm not aware of?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] JaroslavTulach commented on pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
JaroslavTulach commented on pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#issuecomment-723536780
> BTW this helps 'scripting' NetBeans on Windows in general, it's not just a fix for the language server.
Great! As a future follow up I'd suggest to put such scripting integration test into `ant commit-validation`. That way we should get some testing on all platform for free.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] sdedic commented on pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
sdedic commented on pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#issuecomment-723322638
I think I've found yet another race between `killNbProcess` and relaunch of the `nbexec`, visible especially on Windows.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] JaroslavTulach commented on pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
JaroslavTulach commented on pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#issuecomment-723552936
> My personal preference is 12.2 (= release).
Do `12.2` - e.g. release.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] sdedic commented on pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
sdedic commented on pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#issuecomment-723485868
After some collaboration, I've found a solution: if we allow to **inherit stdio handles** to children, the output stream can be monitored for close (`ChildProcess.on('close')`) -- since it closes when the last child in the sequence termintes.
This way the managing js code can safely restart NBJLS server instance when the installation completes.
BTW this helps 'scripting' NetBeans on Windows in general, it's not just a fix for the language server.
I've tested that on Windows 10, nbjavac can be uninstalled from NB instance where it was active/running (so it had locked its JARs) without any issues. According to https://bugs.openjdk.java.net/browse/JDK-7147084?focusedCommentId=13322689&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13322689 Java does not use inheritable handles ... so the change should be moderately safe.
Anyone wanting to re-test the reinstall / remove / replace scenario **on Windows** ? UNIXes are fine, they do not protect opened files from delete/replace.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] sdedic commented on pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
sdedic commented on pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#issuecomment-722435047
I've enhanced however the launchers a bit: if they get exitcode -252 (4), they won't run updater: it is an indication that the java process successfully used `lock` contents to connect to the CLI server and the command(s) were accepted.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] sdedic commented on pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
sdedic commented on pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#issuecomment-723549281
@jtulach @lkishalmi I'm really sorry for the number; I wanted to bump it up, but I simply mistyped the release number and the error passed self-review before commit.
Looking at the version scheme, we seem to had:
* launcher 9.0 at the time of 10.0 release (https://github.com/apache/netbeans/commit/7cac305080542ae7f68f546a5a63a7f9c06bbb40)
* launcher 10.0 at the time of 11.0 release (https://github.com/apache/netbeans/commit/fc9a290b3d7a90cba4af3d04d9573fefff0a5939)
before that, the launcher version was synced with the release version (8.2)
The current fix is not a functional change; so we can either have 10.1 (switches same as 10.0) or 12.2; I couldn't find 12.1 binary:
```
3289B87AB9345958E16F3285ED884F5C4DAB7C2D-platform-launchers-10.0.zip
435688D7DE27C5EE5FAF666D4C6E63DE949ADF0B-platform-launchers-8.2.zip
452B8E1C0922AADEC9D5B63BA8AAEC40FBD2DBAB-platform-launchers-9.0.zip
7A91F97F315B9176838FD15E16C0B9AEC9E6A226-platform-launchers-10.0.zip
A67A12FFB2966457F5FAAE2204720786343C83CC-platform-launchers-9.0.zip
D8A083D00A073359E87B5A077D98541B12E31304-platform-launchers-9.0.zip
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] lkishalmi merged pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
lkishalmi merged pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] sdedic commented on a change in pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
sdedic commented on a change in pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#discussion_r519316643
##########
File path: platform/o.n.bootstrap/launcher/windows/platformlauncher.cpp
##########
@@ -714,7 +720,8 @@ void PlatformLauncher::onExit() {
STARTUPINFO si = {0};
PROCESS_INFORMATION pi = {0};
si.cb = sizeof(STARTUPINFO);
- if (!CreateProcess(NULL, cmdLineStr, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi)) {
+
+ if (!CreateProcess(NULL, cmdLineStr, NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi)) {
Review comment:
@jtulach true observation with UNIX, but ... **unlike UNIX** windows have the obsession of locking opened files. Ideally the subprocess would only receive stdio/out/err handles; but that's not the case, all inheritables are passed. The point of spawning the launcher again and again is to get rid of the locked files (JARs typically) for update operations for in-process JVM.
But sources I've found so far suggest that JVM does not use inheritable handles at all - assuming for security reasons.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] sdedic commented on a change in pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
sdedic commented on a change in pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#discussion_r518819688
##########
File path: platform/o.n.bootstrap/launcher/windows/platformlauncher.cpp
##########
@@ -662,6 +664,10 @@ bool PlatformLauncher::restartRequested() {
void PlatformLauncher::onExit() {
logMsg("onExit()");
+ if (exitStatus == -252) {
Review comment:
! Yes, need to be uploaded to osuosl !
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] JaroslavTulach commented on a change in pull request #2523: Prevent race conditions during CLI install
Posted by GitBox <gi...@apache.org>.
JaroslavTulach commented on a change in pull request #2523:
URL: https://github.com/apache/netbeans/pull/2523#discussion_r519256734
##########
File path: platform/o.n.bootstrap/external/platform-launchers-12.3-license.txt
##########
@@ -1,6 +1,6 @@
Name: NetBeans Application Launchers
Description: Windows Launchers for the NetBeans Platform
-Version: 10.0
+Version: 12.3
Review comment:
I was surprised by the `12.3` number as well. However I was surprised by the previous `10.0` version too. Then I concluded the number doesn't mean much as the release isn't going to use this `ZIP` file at all, but build from the sources.
Sváťa may provide his explanation soon.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists