[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-build-dev
Subject:    Re: RFR: 8168414 Various timeouthandler fixes
From:       Staffan Larsen <staffan.larsen () oracle ! com>
Date:       2016-10-21 8:00:08
Message-ID: 8F0C1E8B-8538-4E10-981C-3901B5AE248B () oracle ! com
[Download RAW message or body]


> On 21 Oct 2016, at 06:10, David Holmes <david.holmes@oracle.com> wrote:
> 
> On 20/10/2016 11:44 PM, Staffan Larsen wrote:
> > 
> > > On 20 Oct 2016, at 15:41, David Holmes <david.holmes@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


[prev in list] [next in list] [prev in thread] [next in thread]