Skip to content

FELIX-6759 Follow-up: Replace Thread.stop() with interrupt() in configadmin UpdateThread - #556

Open
paulrutter wants to merge 10 commits into
fix-module-builds-modern-jdkfrom
fix-configadmin-thread-stop
Open

FELIX-6759 Follow-up: Replace Thread.stop() with interrupt() in configadmin UpdateThread#556
paulrutter wants to merge 10 commits into
fix-module-builds-modern-jdkfrom
fix-configadmin-thread-stop

Conversation

@paulrutter

Copy link
Copy Markdown
Contributor

Split out of #433, where it was unrelated to the Java 25 work it was bundled with.

UpdateThread.terminate() falls back to killing the worker when it fails to join within 5 seconds:

Log.logger.log( LogService.LOG_ERROR,
    "Worker thread {0} did not terminate within 5 seconds; trying to kill", ... );
workerThread.stop();

Thread.stop() has thrown UnsupportedOperationException unconditionally since Java 20, so on any recent JDK this path can only make things worse: instead of the thread being stopped, the exception propagates out of terminate(), turning a slow shutdown into a failed one.

It now interrupts the worker instead, which is the supported mechanism and releases the thread if it is waiting on an interruptible operation. The javadoc and the log message are updated to match.

Found while getting the repository to build on JDK 25, but it is not a Java 25 issue — it has been broken since Java 20.

🤖 Generated with Claude Code

UpdateThread.terminate() falls back to killing the worker when it fails to join
within 5 seconds:

    Log.logger.log( LogService.LOG_ERROR,
        "Worker thread {0} did not terminate within 5 seconds; trying to kill", ... );
    workerThread.stop();

Thread.stop() has thrown UnsupportedOperationException unconditionally since Java 20,
so on any recent JDK that path can only turn a slow shutdown into a failed one: the
exception propagates out of terminate() instead of the thread being stopped.

It now interrupts the worker, which is the supported mechanism and releases the
thread if it is waiting on an interruptible operation. The javadoc and the log
message are updated to match.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@paulrutter
paulrutter changed the base branch from master to feature/FELIX-6759-Java-25-LTS August 29, 2026 21:22
configadmin is not in the workflow's path filters, so a pull request touching only
configadmin matches no trigger and gets no build at all. This change would otherwise
go in unverified.

Adds it to both triggers, to the paths-filter, and as a build step.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@paulrutter paulrutter changed the title Replace Thread.stop() with interrupt() in configadmin UpdateThread FELIX-6759 Follow-up: Replace Thread.stop() with interrupt() in configadmin UpdateThread Aug 29, 2026
@paulrutter
paulrutter changed the base branch from feature/FELIX-6759-Java-25-LTS to fix-module-builds-modern-jdk August 29, 2026 21:38
@paulrutter

Copy link
Copy Markdown
Contributor Author

Restacked onto #555 and narrowed to just the code fix.

CI could not build this: configadmin has the same stale parent defect as the other modules in #555 — it declares felix-parent 6, which does not match the local pom, so Maven resolves the released parent from Central and gets felix.java.version 7:

error: release version 7 not supported

That build repair belongs in #555, which is exactly "make the remaining module builds work on modern JDKs", so it has moved there along with adding configadmin to the CI matrix. This PR now contains only the Thread.stop()interrupt() change and is stacked on #555 so it inherits a working build.

Worth noting separately: configadmin was absent from the workflow's path filters entirely, so this PR initially matched no trigger and received no CI at all rather than a failure. That is a recurring weakness — a module outside the path list gets silence instead of a red build. It has bitten gogo, framework.tck, stacked PR base branches and now configadmin; a catch-all entry or a default that fails loudly would be safer than enumerating every module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant