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

Remove Thread.stop() usage #4952

Open
mbien opened this issue Nov 9, 2022 · 14 comments
Open

Remove Thread.stop() usage #4952

mbien opened this issue Nov 9, 2022 · 14 comments
Labels
Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) kind:bug Bug report or fix priority:high High priority issue that should, if possible, be fixed in next release Upgrade JDK Upgrade to the JDK requirements of a module.

Comments

@mbien
Copy link
Member

mbien commented Nov 9, 2022 •
edited
Loading

Those methods are deprecated since Java 1.2 and will throw UnsupportedOp exceptions from JDK 20+.

got reminded today again by getting this notification:
https://inside.java/2022/11/09/quality-heads-up/
@mbien mbien added kind:bug Bug report or fix priority:high High priority issue that should, if possible, be fixed in next release labels Nov 9, 2022
@BradWalker
Copy link
Member

It is for issues like this, that I've been working on trying to clean up "old code". Because eventually it is going to bite us in a major way..

@neilcsmith-net neilcsmith-net added this to the NB18 milestone Jan 18, 2023
@mbien mbien mentioned this issue Feb 5, 2023
Closed
@matthiasblaesing
Copy link
Contributor

matthiasblaesing commented Feb 10, 2023 •
edited
Loading

I asked jackpot for help and it answered (running the "Invoking Thread.stop()/suspend()/resume()" hint):

nbi/engine/src/org/netbeans/installer/downloader/dispatcher/impl/RoundRobinDispatcher.java:122:
nbi/engine/src/org/netbeans/installer/downloader/dispatcher/impl/RoundRobinDispatcher.java:168:
harness/o.n.insane/src/org/netbeans/insane/scanner/ScannerUtils.java:256:
harness/o.n.insane/src/org/netbeans/insane/scanner/ScannerUtils.java:267:
ide/httpserver/src/org/netbeans/modules/httpserver/HttpServerModule.java:84:
ide/httpserver/src/org/netbeans/modules/httpserver/HttpServerModule.java:151:
ide/httpserver/src/org/netbeans/modules/httpserver/HttpServerModule.java:157:
ide/extbrowser/test/ExtBrowser/qa-functional/src/org/netbeans/test/gui/web/util/HttpRequestWaitable.java:117:
ide/editor.search/test/unit/src/org/netbeans/modules/editor/search/EditorFindSupportTest.java:78:
ide/editor.search/test/unit/src/org/netbeans/modules/editor/search/EditorFindSupportTest.java:112:
ide/editor.search/test/unit/src/org/netbeans/modules/editor/search/EditorFindSupportTest.java:147:
ide/editor.search/test/unit/src/org/netbeans/modules/editor/search/EditorFindSupportTest.java:181:
ide/editor.search/test/unit/src/org/netbeans/modules/editor/search/EditorFindSupportTest.java:217:
ide/editor.search/test/unit/src/org/netbeans/modules/editor/search/EditorFindSupportTest.java:253:
extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/BridgeImpl.java:329:
java/lib.jshell.agent/agentsrc/org/netbeans/lib/jshell/agent/AgentWorker.java:493:
java/java.lsp.server/test/unit/src/org/netbeans/modules/java/lsp/server/protocol/ServerTest.java:263:
webcommon/cordova/test/unit/src/org/netbeans/modules/web/clientproject/cordova/AndroidPlatformTest.java:208:
nb/deadlock.detector/test/unit/src/org/netbeans/modules/deadlock/detector/DetectorTest.java:106:
nb/deadlock.detector/test/unit/src/org/netbeans/modules/deadlock/detector/DetectorTest.java:107:
platform/openide.nodes/test/unit/src/org/openide/nodes/ChildrenKeysTest.java:192:
platform/openide.nodes/test/unit/src/org/openide/nodes/ChildrenKeysTest.java:193:
platform/core.execution/src/org/netbeans/core/execution/SecMan.java:82:
platform/core.execution/src/org/netbeans/core/execution/SecMan.java:84:
platform/openide.util/test/unit/src/org/openide/util/RequestProcessor180386Test.java:479:
platform/openide.util/test/unit/src/org/openide/util/RequestProcessor180386Test.java:504:
platform/o.n.core/src/org/netbeans/core/CLIOptions2.java:113:

This was referenced Feb 18, 2023
Merged
Merged
@mbien
Copy link
Member Author

mbien commented Mar 13, 2023

@matthiasblaesing could you generate a new list so that we can see what is left to do? (or even better share the script which runs jackpot on all projects :))

@matthiasblaesing
Copy link
Contributor

Here you are:

nbi/engine/src/org/netbeans/installer/downloader/dispatcher/impl/RoundRobinDispatcher.java:122:
nbi/engine/src/org/netbeans/installer/downloader/dispatcher/impl/RoundRobinDispatcher.java:168:
extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/BridgeImpl.java:329:
java/lib.jshell.agent/agentsrc/org/netbeans/lib/jshell/agent/AgentWorker.java:493:
java/java.lsp.server/test/unit/src/org/netbeans/modules/java/lsp/server/protocol/ServerTest.java:263:
webcommon/cordova/test/unit/src/org/netbeans/modules/web/clientproject/cordova/AndroidPlatformTest.java:208:
nb/deadlock.detector/test/unit/src/org/netbeans/modules/deadlock/detector/DetectorTest.java:106:
nb/deadlock.detector/test/unit/src/org/netbeans/modules/deadlock/detector/DetectorTest.java:107:
platform/openide.nodes/test/unit/src/org/openide/nodes/ChildrenKeysTest.java:192:
platform/openide.nodes/test/unit/src/org/openide/nodes/ChildrenKeysTest.java:193:
platform/core.execution/src/org/netbeans/core/execution/SecMan.java:82: warning:
platform/core.execution/src/org/netbeans/core/execution/SecMan.java:84: warning:
platform/openide.util/test/unit/src/org/openide/util/RequestProcessor180386Test.java:479:
platform/openide.util/test/unit/src/org/openide/util/RequestProcessor180386Test.java:504:
platform/o.n.core/src/org/netbeans/core/CLIOptions2.java:113:

This list is generated using jackpot like this:

@mbien
Copy link
Member Author

mbien commented Mar 14, 2023

awesome, thanks. This doesn't look that bad anymore thanks to your work. If we remove all tests from that list, we get this:

nbi/engine/src/org/netbeans/installer/downloader/dispatcher/impl/RoundRobinDispatcher.java:122:
nbi/engine/src/org/netbeans/installer/downloader/dispatcher/impl/RoundRobinDispatcher.java:168:
extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/BridgeImpl.java:329:
java/lib.jshell.agent/agentsrc/org/netbeans/lib/jshell/agent/AgentWorker.java:493:
platform/core.execution/src/org/netbeans/core/execution/SecMan.java:82: warning:
platform/core.execution/src/org/netbeans/core/execution/SecMan.java:84: warning:
platform/o.n.core/src/org/netbeans/core/CLIOptions2.java:113:

BridgeImpluses stop as fallback, we could probably simply catch UOE there or guard against JDK 20+.
SecMan is full with deprecated code since SecurityManager itself is deprecated. This will be harder to solve.
AgentWorker for jshell might also cause problems, it should probably interrupt and pray.
SLIOptions2 uses stop too as last resort.

@mbien
Copy link
Member Author

mbien commented Mar 18, 2023 •
edited
Loading

another bogeyman showed up:
https://download.java.net/java/early_access/jdk20/docs/api/java.base/java/lang/ThreadGroup.html#stop()
will throw UOEs too.

https://download.java.net/java/early_access/jdk20/docs/api/java.base/java/lang/ThreadGroup.html#destroy()
is now a no-op.

Tested NB from master on JDK 20 a bit and things did work pretty well already. Canceling background tasks did also work, resetting JShell sessions worked, cancelling maven+ant builds seems to work too, project run -> stop had also no problem.

@neilcsmith-net
Copy link
Member

This is still open and marked high priority for NB18. Where are we at with this? Are there still things we need to address before release?

@mbien
Copy link
Member Author

mbien commented May 2, 2023

@neilcsmith-net we can move this to NB19. This is something we can do incrementally. I couldn't find any showstoppers while testing on JDK 20 so far. Matthias did also replace some important implementations already.

@neilcsmith-net neilcsmith-net modified the milestones: NB18, NB19 May 2, 2023
@mbien mbien added the Upgrade JDK Upgrade to the JDK requirements of a module. label Jun 1, 2023
@mbien mbien modified the milestones: NB19, NB20 Jul 13, 2023
@neilcsmith-net neilcsmith-net removed this from the NB20 milestone Oct 14, 2023
@mbien
Copy link
Member Author

mbien commented Jun 27, 2024

NBI can't be built on JDK 23 since it uses some of the removed methods, example:

   [repeat] /home/runner/work/netbeans/netbeans/nbi/engine/src/org/netbeans/installer/downloader/dispatcher/impl/RoundRobinDispatcher.java:222: error: cannot find symbol
   [repeat]                     current.resume();

https://github.com/apache/netbeans/actions/runs/9690496379

@mbien mbien mentioned this issue Jun 27, 2024
Merged
@mbien
Copy link
Member Author

mbien commented Jun 27, 2024

instead of fixing this, should we get rid of the downloader package+functionality?

@BradWalker
Copy link
Member

instead of fixing this, should we get rid of the downloader package+functionality?

I am sill a believer that we should clearn up the usage of Thread.stop instead of removing the downloader package?

@mbien
Copy link
Member Author

mbien commented Jul 3, 2024

the experiment at #7538 showed that removal of downloader/* is likely the quickest option. It builds, it tests and I also generated an .exe installer and tested it manually (only on JDK 22 since the (native) installer can't parse ea java versions, but this is off topic).

NBI is in bad shape, unmaintained and NB is slowly transitioning to @neilcsmith-net's nbpackage, everything except windows is using it already. The NBI issue is preventing us from building or testing NetBeans on JDK 23 which is a problem.

I will open a proper PR soon, with the cleaned up changes which remove the online functionality from NBI to give it a small life time extension. This would go in first, then we can merge the nb-javac 23 PR once ready, then the CI-on-23 PR and we are (hopefully) good for the NB 23 cycle.

@mbien mbien mentioned this issue Jul 3, 2024
Merged
@mbien
Copy link
Member Author

mbien commented Jul 3, 2024

see #7542 for NBI update

@matthiasblaesing
Copy link
Contributor

instead of fixing this, should we get rid of the downloader package+functionality?

I am sill a believer that we should clearn up the usage of Thread.stop instead of removing the downloader package?

This was my first reaction and as @mbien writes in #7542 also his. But this needs someone to actually do the work and I decided for myself that I'm not willing to do it.

@mbien mbien added the Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) label Feb 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) kind:bug Bug report or fix priority:high High priority issue that should, if possible, be fixed in next release Upgrade JDK Upgrade to the JDK requirements of a module.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mbien @BradWalker @matthiasblaesing @neilcsmith-net