Skip to content

cleanup: remove deprecated TheadGroup methods #9091

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

Closed
BradWalker wants to merge 1 commit into from
+6 −43
Closed

cleanup: remove deprecated TheadGroup methods #9091

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 0 additions & 31 deletions platform/core.execution/src/org/netbeans/core/execution/DefaultSysProcess.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,35 +101,4 @@ public void run() {
public String getName() {
return name;
}

/** destroy the thread group this process was handled from. Not that simple
* as it seems, since the ThreadGroup can't be destroyed from inside.
*/
void destroyThreadGroup(ThreadGroup base) {
new Thread(base, new Destroyer(group)).start();
}
private static class Destroyer implements Runnable {
private final ThreadGroup group;
Destroyer(ThreadGroup group) {
this.group = group;
}
@Override public void run() {
try {
while (group.activeCount() > 0) {
Thread.sleep(1000);
}
}
catch (InterruptedException e) {
Exceptions.printStackTrace(e);
}
if (!group.isDestroyed()) {
try {
group.destroy();
} catch (IllegalThreadStateException x) {
// #165302: destroyed some other way?
}
}
}
}

}
12 changes: 6 additions & 6 deletions platform/core.execution/src/org/netbeans/core/execution/ExecutionEngine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@

/* table of window:threadgrp */
private static final WindowTable wtable = new WindowTable();

/** list of ExecutionListeners */
private final HashSet<ExecutionListener> executionListeners = new HashSet<>();
private final Set<ExecutionListener> executionListeners;

/** List of running executions */
private final List<ExecutorTask> runningTasks = Collections.synchronizedList(new ArrayList<>(5));
Expand All @@ -87,6 +87,8 @@
static final long serialVersionUID =9072488605180080803L;

public ExecutionEngine () {
this.executionListeners = new HashSet<>();

/* SysIn is a class that redirects System.in of some running task to
a window (probably OutWindow).
SysOut/Err are classes that redirect out/err to the window
Expand Down Expand Up @@ -134,7 +136,6 @@ public String getRunningTaskName( ExecutorTask task ) {
@Override
public ExecutorTask execute(String name, Runnable run, final InputOutput inout) {
TaskThreadGroup g = new TaskThreadGroup(base, "exec_" + name + "_" + number); // NOI18N
g.setDaemon(true);
ExecutorTaskImpl task = new ExecutorTaskImpl();
synchronized (task.lock) {
try {
Expand Down Expand Up @@ -194,7 +195,7 @@ protected final PermissionCollection createPermissions(CodeSource cs, InputOutpu
protected final void fireExecutionStarted (ExecutionEvent ev) {
runningTasks.add( ev.getProcess() );
@SuppressWarnings("unchecked")
Iterator<ExecutionListener> iter = ((HashSet<ExecutionListener>) executionListeners.clone()).iterator();
Iterator<ExecutionListener> iter = executionListeners.iterator();
Comment on lines -197 to +198
Copy link
Contributor

@matthiasblaesing matthiasblaesing Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes me nervous. Why is the removal of the clone safe? HashMaps are not synchronized and if I remember correctly iteration of a map that is modified by another thread is undefined behavior. The clone removes the need for synchronization (at least I would read it as such).

Copy link
Member Author

@BradWalker BradWalker Dec 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes me nervous. Why is the removal of the clone safe? HashMaps are not synchronized and if I remember correctly iteration of a map that is modified by another thread is undefined behavior. The clone removes the need for synchronization (at least I would read it as such).

Had to look into this more. You are correct. I updated this so that a synchronizedSet is used. This seems to be a better choice compared to making a copy of the Set. The code is cleaner and correct.

Good eyes. Thanks for catching this. I learned something here..

while (iter.hasNext()) {
ExecutionListener l = iter.next();
l.startedExecution(ev);
Expand All @@ -205,12 +206,11 @@ protected final void fireExecutionStarted (ExecutionEvent ev) {
protected final void fireExecutionFinished (ExecutionEvent ev) {
runningTasks.remove( ev.getProcess() );
@SuppressWarnings("unchecked")
Iterator<ExecutionListener> iter = ((HashSet<ExecutionListener>) executionListeners.clone()).iterator();
Iterator<ExecutionListener> iter = executionListeners.iterator();
while (iter.hasNext()) {
ExecutionListener l = iter.next();
l.finishedExecution(ev);
}
ev.getProcess().destroyThreadGroup(base);
}

static void putWindow(java.awt.Window w, TaskThreadGroup tg) {
Expand Down
6 changes: 0 additions & 6 deletions platform/openide.util/src/org/openide/util/RequestProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1892,20 +1892,14 @@ static Processor get() {
return proc;
}
} else {
assert checkAccess(TOP_GROUP.getTopLevelThreadGroup());
Processor proc = POOL.pop();
proc.idle = false;

return proc;
}
}
newP = new Processor();
}
}
private static boolean checkAccess(ThreadGroup g) throws SecurityException {
g.checkAccess();
return true;
}

/** A way of returning a Processor to the inactive pool.
*
Expand Down
Loading