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

[David Holmes]

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
>