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

[Seán Coffey]

Looks ok to me too Jaraslav,

Corba tests are sadly lacking. I can run this through a testsuite which 
QA team use.

regards,
Sean.

On 04/10/2012 16:42, Alan Bateman wrote:
> On 04/10/2012 16:28, Jaroslav Bachorik wrote:
>> :
>> This is a follow-up. I've prepared the patch and put it on github -
>> https://github.com/jbachorik/openjdk-patches/tree/master/webrevs/7195779
>>
>> I wonder who else should be included in the review process since I am
>> changing the IIOP generator code. Also, I didn't find any tests in the
>> corba repository. Which test suite is appropriate to run after changing
>> the corba related code?
>>
>> -JB-
>>
> I don't mind being reviewer and sponsor for this. Also cc'ing Sean as 
> he is one of the maintainers of the corba code. I don't think the 
> corba tests are in OpenJDK, at least I don't think Oracle has 
> contributed its tests for this area.
>
> I think your change looks okay and I assume you've at least run the 
> JMX tests that use RMI-IIOP to verify that the intermittent NPE is 
> gone and those tests now pass reliably.
>
> Minor comment but if I were doing this myself then I probably would 
> have added this instead:
> p.pln(getName(theType) + " target = this.target;");
>
> You'll see lots of examples of this in the core libs and j.u.c.
>
> Also as target is now volatile then I'm not sure why you synchronized 
> around target=null, perhaps there is other code generated in the tie 
> class that I don't see?
>
> Otherwise it's great to get issue finally resolved.
>
> -Alan.
>
>