Review Request for CR : 7144861 RMI activation tests are too slow (original) (raw)

David Holmes david.holmes at oracle.com
Wed May 9 03:53:03 UTC 2012

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

More information about the core-libs-dev mailing list