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
