Review Request for CR : 7144861 RMI activation tests are too slow

[David Holmes]

On 9/05/2012 12:53 PM, Stuart Marks wrote:
> On 5/7/12 10:32 PM, David Holmes wrote:
>> As a general principle library code that catches IE and doesn't
>> rethrow it
>> should re-assert the interrupt state.
>>
>> But this isn't library code it is a stand-alone test framework as I
>> understand
>> it. So if the framework doesn't use interruption at all then the
>> response is
>> somewhat arbitrary. As you point out this code is all over the place with
>> regard to IE handling at present. The clean solution would assume
>> interrupts
>> were used to signify cancellation and code accordingly, but doing that
>> consistently all through may be a larger cleanup than Olivier wants to
>> try now.
>
> This is indeed test code, but just as library code should be decoupled
> from the caller, test code should be decoupled from its framework. So I
> think the general principle applies.
>
> I'm not asking that the all code be audited to make sure handles IE
> properly everywhere. But Olivier's changes did touch some code that
> handles IE, which naturally raises the question of the proper way to
> handle IE. Reasserting the interrupt bit and continuing around the loop
> just causes it to spin until the loop is exhausted, which doesn't seem
> sensible.

Right - that certainly has to be changed.

> Returning early seems sensible. Applying the rule says that
> the code should reassert the interrupt bit before returning.

If interruptible code is looping around an interruptible method that 
must be re-executed then the usual approach is to re-assert the 
interrupt outside the loop eg:

boolean wasInterrupted = false;
while (cond) {
   try {
       interruptibleOp();
   }
   catch (InterruptedException ie) {
     wasInterrupted = true;
     continue;
   }
}
if (wasInterrupted)
   Thread.currentThread().interrupt();

In this case however this seems more like a fail-fast situation so 
immediate return seems in order - though the fact the test was 
interrupted does have to be reported to the test harness somehow.

David
-----

> An alternative would be to have the code rethrow or propagate IE, but
> this requires additional refactoring and probably adding throws clauses
> to methods, so it's probably not warranted.
>
> My recommendation to Olivier is to run through the files in the
> changeset and modify the catch blocks as follows:
>
> catch (InterruptedException ie) {
> Thread.currentThread().interrupt();
> // maybe print a log message
> return; // or break, as appropriate
> }
>
> s'marks