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
