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. Returning early seems sensible.
Applying the rule says that the code should reassert the interrupt bit before
returning.
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
