Bug 567345 - RuntimeProcess should terminate descendants (requires at least Java-9)
Summary: RuntimeProcess should terminate descendants (requires at least Java-9)
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Debug (show other bugs)
Version: 4.17   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 4.18 M3   Edit
Assignee: Hannes Wellmann CLA
QA Contact:
URL:
Whiteboard:
Keywords: noteworthy
: 77082 (view as bug list)
Depends on: 570480
Blocks:
  Show dependency tree
 
Reported: 2020-09-25 06:04 EDT by Hannes Wellmann CLA
Modified: 2021-01-19 08:14 EST (History)
3 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Hannes Wellmann CLA 2020-09-25 06:04:09 EDT 
When org.eclipse.debug.core.model.RuntimeProcess.terminate() is called, only the wrapped java.long.Process is destroyed. Sub-processes created by the wrapped process are left untouched.
This can cause bad situations if, for example a created sub-process locks any kind of resources and continues running after the main-process has terminated.

Prior Java-9 finding all sub-process (descendents) of a process was not possible with the Java standard library.
Since the extension of the Process API in Java-9 the Process class provides the method Process.descendants() to get a stream of all sub-processes (and their sub-process, recursively).
This can be used to fetch all sub-processes prior termination and to destroy them after the main-process.

I can provide a patch if you are interested.

If I see it correctly the affected org.eclipse.debug.core plug-in still requires Java-8. So this has to be updated first. But because since Eclipse 4.17/2020-09 Java-11 is required to run Eclipse, this should be possible.

This a generalization of Bug 77082
Comment 1 Sarika Sinha CLA 2020-09-25 06:30:36 EDT 
Yes, the plugin can move to 11 compliance.
Please provide a gerrit.
Comment 2 Eclipse Genie CLA 2020-09-25 08:39:04 EDT 
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.debug/+/169905
Comment 3 Hannes Wellmann CLA 2020-09-25 08:50:27 EDT 
I submitted a commit to Gerrit containing the bug-fix and the move to Java-11.

When terminated a RuntimeProcess could also wait for the descendants to terminate. But since ProcessHandle has not waitFor() method it could only be done by polling ProcessHandle.isAlive().
Do you think is is acceptable? Or should RuntimeProcess simply not wait for descendants?

For the Java version migration I just replaced all version entries. When this is done via the Java Build-Path section in the projects preference-Editor some more entries are added. Maybe somebody else has to evaluate this.
Comment 4 Eclipse Genie CLA 2020-09-26 09:11:58 EDT 
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.debug/+/169938
Comment 5 Eclipse Genie CLA 2020-09-29 05:07:30 EDT 
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.debug/+/170009
Comment 7 Sarika Sinha CLA 2020-10-28 00:53:05 EDT 
Thanks Hannes for the contribution.
Comment 8 Andrey Loskutov CLA 2020-10-28 09:53:28 EDT 
*** Bug 77082 has been marked as a duplicate of this bug. ***
Comment 9 Sarika Sinha CLA 2020-10-29 00:54:51 EDT 
I missed the another work in progress gerrit.
Comment 12 Paul Pazderski CLA 2020-11-09 15:20:00 EST 
(In reply to Sarika Sinha from comment #9)
> I missed the another work in progress gerrit.

Which are now merged as well. Thanks Hannes.
Comment 13 Sarika Sinha CLA 2020-11-17 05:24:14 EST 
@Hannes,
Please add an entry to N&N as we need to inform the users that we have changed the termination method.
Comment 14 Hannes Wellmann CLA 2020-11-17 11:06:27 EST 
@Sarika
Sure I can do that, but I don't know how. Can you please give me some guidance.
Comment 15 Sarika Sinha CLA 2020-11-17 12:10:58 EST 
(In reply to Hannes Wellmann from comment #14)
> @Sarika
> Sure I can do that, but I don't know how. Can you please give me some
> guidance.
This is the repository: 
www.eclipse.org/eclipse/news.git

Entry will go in Platform Debug Category. You will find Instructions.html helpful.
Comment 16 Eclipse Genie CLA 2020-11-22 15:54:07 EST 
New Gerrit change created: https://git.eclipse.org/r/c/www.eclipse.org/eclipse/news/+/172644
Comment 17 Hannes Wellmann CLA 2020-11-22 15:57:53 EST 
Thanks for the instructions, I just submitted an entry for the platform.html.
I hope it is suitable.
Comment 19 Sarika Sinha CLA 2020-11-24 00:38:21 EST 
(In reply to Hannes Wellmann from comment #17)
> Thanks for the instructions, I just submitted an entry for the platform.html.
> I hope it is suitable.

Thanks!
Comment 20 Andrey Loskutov CLA 2020-12-16 15:11:42 EST 
(In reply to Eclipse Genie from comment #11)
> Gerrit change
> https://git.eclipse.org/r/c/platform/eclipse.platform.debug/+/170009 was
> merged to [master].
> Commit:
> http://git.eclipse.org/c/platform/eclipse.platform.debug.git/commit/
> ?id=9223452bf28e2ec766da65d4530e6e0921c90548

This one caused severe regression, see bug 569750 :-(.