RFR: JDK-8240148: Uncaught exceptions from ScheduledThreadPoolExecutor.execute aren't reported

Jaikiran Pai jai.forums2013 at gmail.com
Wed Jun 24 13:44:34 UTC 2020


Could I please get a review and a sponsor for a fix for
https://bugs.openjdk.java.net/browse/JDK-8240148?

The patch is available as a webrev at
https://cr.openjdk.java.net/~jpai/webrev/8240148/1/

The patch updates the ScheduledThreadPoolExecutor.execute(Runnable) to
wrap the user passed Runnable into a custom one which then catches any
Throwable and reports it through the Thread's uncaught exception
handler. After having reviewed the existing code in that class, this was
the easiest (and perhaps the only feasible?) and cleanest way that I
could think of, to fix this issue.

A few things about this change:

1. I decided to use an anonymous class for the Runnable, in the
execute() method instead of a lambda because I was unsure if this part
of the JDK code is allowed to use a lambda. In the past I have seen
discussions where it was recommended not use lambda in certain parts of
the JDK either because of performance(?) reason or because "this part of
the code runs too early to be using a lambda" reason. I personally
prefer an anonymous class anyway, but if others feel using a lambda here
is relevant, then let me know.

2. Once the exception is reported using the UncaughtExceptionHandler, I
just return. Initially, I had considered throwing back the original
exception/throwable after reporting it via the UncaughtExceptionHandler,
but having looked at few others places within the JDK which do not throw
back the original exception, I decided to follow the same semantics.

3. Now that the passed Runnable is wrapped and submitted for execution,
I considered whether this would break any (unwritten) contract between
the ThreadPoolExecutor and the ThreadFactory#newThread(Runnable)
implementations. If any (JDK internal or user application specific)
ThreadFactory#newThread method was expecting the passed Runnable to be
of the same type or the same instance as that was submitted to the
ScheduledThreadPoolExecutor#execute(Runnable), then this change would
break such ThreadFactory#newThread implementations. I looked deeper in
the ThreadPoolExecutor code and from what I could see, this isn't a
practical concern, since even without this change, The
ThreadPoolExecutor in its private Worker class already sends a
"delegate" Runnable (an instance of the
ThreadPoolExecutor$Worker class) to the ThreadFactory's newThread method
and not the original Runnable that was submitted to the
execute(Runnable) method of the executor. So it doesn't look like
there's any written/unwritten contract that the ThreadFactory is
expected to receive the same Runnable instance that was passed to the
execute method of the executor.

4. The ScheduledThreadPoolExecutor has a different license than some of
the other files that I am used to, within the JDK code. So I haven't
edited it for any of the usual copyright year updates.

5. The patch contains a new (testng based) test case which reproduces
this issue and verifies the fix.

-Jaikiran




More information about the core-libs-dev mailing list