Review Request: 7195779 javax/management/remote/mandatory/threads/ExecutorTest.java fail intermittently

[Jaroslav Bachorik]

On Fri 21 Sep 2012 12:52:27 PM CEST, Alan Bateman wrote:
> On 20/09/2012 17:02, Eamonn McManus wrote:
>> Changing the generated RMI/IIOP code
>> so that it no longer causes this exception, or so that it catches it
>> and rethrows a RemoteException, sounds as if it ought to be fairly
>> straightforward, and that's probably what I would do if it were up to
>> me.
> I think this is what I would do to, even though it means going into
> the corba repository as that is there the stub generator is. I should
> say that I don't violently object to Jaroslav's patch, it's just that
> it is an ugly workaround.

The generated TIE class is inherently thread-unsafe. The internal state 
(the target field) can be manipulated without any enforced 
synchronization - eg. it is valid to set the target field to null by 
calling the deactivate() method after the _invoke() method has been 
entered from a different thread. This will lead to the NPE we can 
observe. Given this example one should make critical sections out of 
deactivate() and _invoke() methods to prevent this situation. However, 
this simplistic approach might lead to deadlocks in the existing code 
as the _invoke() method body might be blocking (it is a 3rd party code) 
and thus preventing execution of the deactivate() method indefinitely.

Also, it is not really possible to solve this problem outside of the 
generated TIE class - it is caused by the concurrent change of the 
TIE's internal state. So, the solution would be either caching the 
target attribute at the beginning of the invoke() operation in a 
synchronized block and use the cached version afterwards (and throwing 
a remote exception if it is null - the TIE was deactivated effectively 
before entering the invoke() operation) or postponing deactivation when 
the invoke() method is detected as being in progress.

-JB-

>
>
>> Disabling this test for the IIOP case, and probably other failing
>> JMX tests that involve IIOP, is an option if it is judged that nobody
>> uses the RMI/IIOP connector any more so it is all right to let it rot.
>> That judgement is a non-technical one that I don't have an informed
>> opinion on.
>>
> I don't know if the rmi-iiop connector was ever used much but since it
> seems to be required by the JMX Remote API spec then I think we should
> continue to test it. As I think I mentioned in another mail recently,
> I think we have to look at making this transport optional as it's
> painful to have the CORBA tie/stub classes in
> javax.management.remote.rmi. I don't what to hijack Jaroslav's thread
> to discuss that, that's a topic for another thread.
>
> -Alan
>