> On 21 Oct 2016, at 06:10, David Holmes <david.hol...@oracle.com> wrote: > > On 20/10/2016 11:44 PM, Staffan Larsen wrote: >> >>> On 20 Oct 2016, at 15:41, David Holmes <david.hol...@oracle.com> wrote: >>> >>> On 20/10/2016 11:27 PM, Staffan Larsen wrote: >>>> When looking for some timeout handler problems I found a few things that >>>> should be fixed: >>>> >>>> * Strange use of Thread.currentThread().interrupt() >>> >>> That isn't "strange" it is an idiomatic usage - if you can't propagate the >>> InterruptedException to show your caller you have been interrupted then you >>> re-assert the interrupt state. >> >> OK, then. In this case it is less “strange” than “wrong”. The cases where >> this is done should not propagate the interrupt state - it only serves to >> throw unexpected exceptions higher up in the call-chain. > > If code higher up the call chain can not handle being interrupted and > interrupt is being used within the program to signal cancellation, then it is > the higher up code that has a problem. As I said the existing code is an > idiomatic approach to dealing with cancellation requests.
jtreg uses interrupt() to try to signal to the timeout handler that it has exceeded its timeout (-timeoutHandlerTimeout). This is the default way for jtreg to cancel timeout handlers. There are a couple of issues with this. First, interrupt() does not work well as a way to cancel I/O work (which is what the timeout handler is doing most of the time). Second, this particular timeout handler does a very good job of handling timeouts on its own. I will soon send out a patch to remove jtreg’s timeout when running with our timeout handler, but even with that patch there are cases when jtreg calls interrupt() on the timeout handler. In those cases we just want to ignore that and keep on doing our work, taking the liberty of saying that we know better than jtreg. /Staffan > > Cheers, > David > >>> >>> Cheers, >>> David >>> >>>> * Add logging for timeouts >>>> * Milliseconds sometimes printed as microseconds or nanoseconds >>>> * Should use destroyForcibly() to terminate processes >>>> * No need to sleep in the last iteration when running a command multiple >>>> times >>>> * Disable timeout handling timeouts since we do that ourselves >>>> >>>> I didn’t want to file individual bugs for all of these, so I have lumped >>>> them together in one bug. >>>> >>>> bug: https://bugs.openjdk.java.net/browse/JDK-8168414 >>>> <https://bugs.openjdk.java.net/browse/JDK-8168414>webrev: >>>> http://cr.openjdk.java.net/~sla/8168414/webrev.00 >>>> <http://cr.openjdk.java.net/~sla/8168414/webrev.00> >>>> >>>> Thanks, >>>> /Staffan
