RFR : JDK-6744127 - NullPointerException at com.sun.tools.jdi.EventRequestManagerImpl.request

[Harsha Wardhana B]

Hi All,

Please find below the updated webrev incorporating review comments.

Issue : https://bugs.openjdk.java.net/browse/JDK-6744127
webrev : 
http://cr.openjdk.java.net/~jbachorik/sponsorship/6744127/webrev.01/

Hello Serguei,

I will send fix for tonga test case in a separate review request.

Regards
Harsha

On Tuesday 15 December 2015 01:31 PM, serguei.spitsyn at oracle.com wrote:
> Hi Harsha,
>
> The fix looks good in general.
>
> If I understand correctly, any invoke of the requestList().add(), 
> requestList().remove()
> or requestList().clear() will be implicitly synchronized on the list 
> object.
> There only place left where we still need an explicit synchronized 
> statement
> is the iteration over the list in the body of the method:
>   EventRequest request(int eventCmd, int requestId);
>
> I have some minor comments, though.
>
>   The comment at the line 936 needs a dot at the end.
>   A space is needed after the keywords "synchronized", "while" and 
> "if" (lines: 943, 945 and 947).
>   The space is not needed at the line 946 after the cast.
>
>   Also, I'd use the for-loop instead of while-loop like this:
>   for (EventRequestImpl er: rl) {
>        if (er.id == requestId) {
>              return er;
>         }
>    }
>
>    But I leave it up to to you if you like to keep your variant.
>
>    What tests did you run to make sure the fix does not brake anything?
>    I'd recommend to run the JTreg com/sun/jdi tests and the co-located 
> nsk.jdi.testlist tests.
>    If you already done so, could you, please, share the results?
>
>
> Thank you for fixing this bug!
> Serguei
>
>
>
> On 12/13/15 23:32, Harsha Wardhana B wrote:
>> Hi All,
>>
>> Please review the fix for bug,
>>
>> Issue : https://bugs.openjdk.java.net/browse/JDK-6744127
>> Webrev : 
>> http://cr.openjdk.java.net/~jbachorik/sponsorship/6744127/webrev.00
>>
>> The fix synchronizes access to requestList of EventRequestManagerImpl.
>>
>> Thanks
>> Harsha
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20151221/ff68085f/attachment.html>