fskorgen opened a new issue, #8228:
URL: https://github.com/apache/hop/issues/8228
### Apache Hop version?
2.19
### Java version?
21
### Operating system
Windows
### What happened?
Three database dialogs start a background thread that watches for the user
pressing Cancel and
then kills the running query. Each waits like this:
```java
while (pmd.getShell() == null
|| (!pmd.getShell().isDisposed() && !monitor.isCanceled())) {
Thread.sleep(100); // 250 in GetQueryFieldsProgressDialog
}
```
`pmd.getShell() == null` is meant to cover the moment before the dialog's
shell exists. In Hop Web
that moment never ends. `ProgressMonitorDialog.run` returns before the shell
is ever built:
```java
public void run(boolean cancelable, IRunnableWithProgress runnable)
throws InvocationTargetException, InterruptedException {
if (EnvironmentUtils.getInstance().isWeb()) {
runnable.run(new ProgressNullMonitorListener());
return;
}
createModalShell(cancelable);
...
```
So `getShell()` stays `null` for the lifetime of the process, the first
clause of the loop condition
is permanently true, and the watcher polls every 100–250 ms forever. The
second clause cannot rescue
it either: the watcher polls `pmd.getProgressMonitor()`, which is the
dialog's own monitor, while the
runnable was handed a `ProgressNullMonitorListener` — and with no shell
there is no Cancel button to
set the flag in the first place.
One leaked thread per call, for the life of the server.
### Severity differs per class
| Class | Thread | Ends in web? |
|---|---|---|
| `GetQueryFieldsProgressDialog:104` | `new Thread(run).start()` — platform,
**non-daemon** | never |
| `GetTableSizeProgressDialog:116` | `new Thread(run).start()` — platform,
**non-daemon** | never |
| `SqlEditor:329` | `Thread.ofVirtual()` — virtual, daemon | never, but does
not block shutdown |
The two non-daemon ones are the serious case: besides burning a thread each,
a live non-daemon
thread keeps the JVM from exiting normally.
### GetPreviewTableProgressDialog already does it right
The fourth dialog in the same package is the counter-example, and shows the
intended shape:
```java
public List<Object[]> open() {
...
if (!EnvironmentUtils.getInstance().isWeb()) {
openDesktop(); // starts the watcher
} else {
openWeb(); // busy cursor, no watcher
}
```
Its watcher is also the only one that is named and marked daemon:
```java
Thread cancelWatcher = new Thread(run, "Hop-DB-Preview-CancelWatcher");
cancelWatcher.setDaemon(true);
```
So the pattern to copy already exists next door — a web branch that never
starts a watcher, and a
daemon thread with a name for the desktop branch.
### Steps to reproduce
1. Run Hop Web.
2. Trigger any of the three: get the fields of a query
(`GetQueryFieldsProgressDialog`), show a table
row count (`GetTableSizeProgressDialog`), or execute SQL in the SQL
editor.
3. Take a thread dump, or watch the thread count.
**Expected:** no watcher thread outlives the operation — there is no Cancel
button in web to watch
for.
**Actual:** one thread per call, still looping. From
`GetQueryFieldsProgressDialog` and
`GetTableSizeProgressDialog` they are non-daemon `Thread-N` with no name to
identify them by.
### Suggested fix
Two independent changes, either of which stops the leak:
1. **Do not start a watcher when there is nothing to watch** — the guard
`GetPreviewTableProgressDialog` already uses:
```java
if (EnvironmentUtils.getInstance().isWeb()) {
return; // run() executes inline, no shell, no Cancel button
}
```
2. **Make the loop terminate on its own.** The `getShell() == null` clause
assumes the shell will
appear; bounding it (give up after a few seconds, or check a "started"
flag the dialog sets)
removes the assumption for every future caller.
Worth doing regardless: name the threads and mark them daemon, as
`GetPreviewTableProgressDialog:157` does. An unnamed non-daemon `Thread-N`
in a thread dump is hard
to attribute, and daemon status alone would stop these from holding up
shutdown.
### Issue Priority
Priority: 2
### Issue Component
Component: Hop Web
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
