On 8/05/2012 8:29 AM, Stuart Marks wrote:
I think we agree that there's a lot more cleanup that could be done
here, but that we should just press forward with this and postpone the
cleanup until later. I don't really like postponing things but I don't
want to hold up the nice optimizations you've done.
Right - some things take too much cleaning :)
I do have one point to resolve, however, regarding InterruptedException:
On 5/7/12 10:35 AM, Olivier Lagneau wrote:
David Holmes said on date 5/6/2012 2:30 PM:
On 5/05/2012 11:50 AM, Stuart Marks wrote:
If we catch an InterruptedException while sleeping, we reassert the
interrupt bit by interrupting the current thread. This is usually good
practice. In this case though we go around the loop again, retry, and
sleep again. But if we call sleep() when the interrupt bit is set, it
returns immediately. As it stands, after being interrupted, this code
will retry a bunch of times effectively without sleeping and will then
return false.
I guess if we're going to go to the trouble of reasserting the
interrupt
bit we might as well return false immediately.
Does anything in this testing framework actually interrupt any threads?
Elsewhere in this test the interrupt, should it be possible, just
skips to
the next loop iteration without being re-asserted. So either we need to
re-assert always or never - and I suspect never.
Agreed. I think we should just ignore these interrupts since I don't
think any
rmi test of code in framework
might interrupts threads running that code. So thin we never need to
re-assert.
There are several places in the existing code where it catches IE and
re-asserts the interrupt bit. This makes sense for cases like
ActivationLibrary.safeDestroy() where the only thing it can do is return
(i.e., there's no loop here). The caller can distinguish the interrupted
case from the normal case by checking the thread's interrupt bit.
In other cases the existing code catches IE and either continues around
the loop (ActivationLibrary.deactivate) or reasserts the interrupt bit
before continuing around the loop (e.g., RMID.start). I think both of
these are wrong. Basically these loops are waiting for something to
happen. If the code gets an IE it should terminate the loop. The only
question in my mind is whether it should reassert the interrupt bit
before doing so.
David, I know you know this stuff pretty well, what do you think?
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.
David
-----
Offhand I don't know whether this test framework uses interruption or
not. If it doesn't, it seems reasonable that it might be added in the
future. It just bugs me that there's code to catch IE that just ignores
it. I don't think we should go out of our way to do something special in
the case of IE, but returning or breaking out of the loop instead of
ignoring IE seems pretty sensible.
BTW a good background article on threads and interrupts is here: [1].
s'marks
[1] http://www.ibm.com/developerworks/java/library/j-jtp05236/index.html
