Quantcast

[8u] RFR for JDK-8168914: Crash in ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking? to jdk8u-dev

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[8u] RFR for JDK-8168914: Crash in ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking? to jdk8u-dev

Shafi Ahmad
Hi,
 
Please review the backport of bug: "JDK-8168914: Crash in ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking" to jdk8u-dev

Please note that this is not a clean backport due to -
A) Method  remove_handle(jobject h) is not present in jdk8u and looks it is related to module so I haven't merged associated change to jdk8u.
B) File moduleEntry.cpp is not present in jdk8u repo, so I have ignore its changes.


Webrev: http://cr.openjdk.java.net/~shshahma/8168914/webrev.00/
jdk9 bug: https://bugs.openjdk.java.net/browse/JDK-8168914
Original patch pushed to jdk9: http://hg.openjdk.java.net/jdk9/hs/hotspot/rev/606c35b6fac5

Test:  Run jprt and jtreg is in progress.

Regards,
Shafi
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [8u] RFR for JDK-8168914: Crash in ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking? to jdk8u-dev

David Holmes
Hi Shafi,

On 26/04/2017 3:32 PM, Shafi Ahmad wrote:
> Hi,
>
> Please review the backport of bug: "JDK-8168914: Crash in ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking" to jdk8u-dev
>
> Please note that this is not a clean backport due to -
> A) Method  remove_handle(jobject h) is not present in jdk8u and looks it is related to module so I haven't merged associated change to jdk8u.

Without remove_handle_unsafe there is no use of "contains" so all of the
code in the block:

  138 #ifdef ASSERT
  139 class VerifyContainsOopClosure : public OopClosure {
  ...
  167 #endif

is dead code and can be deleted.

Otherwise the backported code looks okay to me.

Thanks,
David
-----

> B) File moduleEntry.cpp is not present in jdk8u repo, so I have ignore its changes.
>
>
> Webrev: http://cr.openjdk.java.net/~shshahma/8168914/webrev.00/
> jdk9 bug: https://bugs.openjdk.java.net/browse/JDK-8168914
> Original patch pushed to jdk9: http://hg.openjdk.java.net/jdk9/hs/hotspot/rev/606c35b6fac5
>
> Test:  Run jprt and jtreg is in progress.
>
> Regards,
> Shafi
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RE: [8u] RFR for JDK-8168914: Crash in ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking? to jdk8u-dev

Shafi Ahmad
Hi David,

Thank you for the review comment.

I have deleted the redundant code from the cpp and hpp file under #ifdef ASSERT ... #endif
Please find updated webrev - http://cr.openjdk.java.net/~shshahma/8168914/webrev.01/

Regards,
Shafi

> -----Original Message-----
> From: David Holmes
> Sent: Wednesday, April 26, 2017 12:09 PM
> To: Shafi Ahmad <[hidden email]>; hotspot-
> [hidden email]
> Cc: Coleen Phillimore <[hidden email]>; Thomas Schatzl
> <[hidden email]>; Stefan Karlsson
> <[hidden email]>
> Subject: Re: [8u] RFR for JDK-8168914: Crash in
> ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking? to
> jdk8u-dev
>
> Hi Shafi,
>
> On 26/04/2017 3:32 PM, Shafi Ahmad wrote:
> > Hi,
> >
> > Please review the backport of bug: "JDK-8168914: Crash in
> > ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking" to
> > jdk8u-dev
> >
> > Please note that this is not a clean backport due to -
> > A) Method  remove_handle(jobject h) is not present in jdk8u and looks it is
> related to module so I haven't merged associated change to jdk8u.
>
> Without remove_handle_unsafe there is no use of "contains" so all of the
> code in the block:
>
>   138 #ifdef ASSERT
>   139 class VerifyContainsOopClosure : public OopClosure {
>   ...
>   167 #endif
>
> is dead code and can be deleted.
>
> Otherwise the backported code looks okay to me.
>
> Thanks,
> David
> -----
>
> > B) File moduleEntry.cpp is not present in jdk8u repo, so I have ignore its
> changes.
> >
> >
> > Webrev: http://cr.openjdk.java.net/~shshahma/8168914/webrev.00/
> > jdk9 bug: https://bugs.openjdk.java.net/browse/JDK-8168914
> > Original patch pushed to jdk9:
> > http://hg.openjdk.java.net/jdk9/hs/hotspot/rev/606c35b6fac5
> >
> > Test:  Run jprt and jtreg is in progress.
> >
> > Regards,
> > Shafi
> >
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [8u] RFR for JDK-8168914: Crash in ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking? to jdk8u-dev

Erik Helin-2
On 04/26/2017 09:43 AM, Shafi Ahmad wrote:
> Hi David,
>
> Thank you for the review comment.
>
> I have deleted the redundant code from the cpp and hpp file under #ifdef ASSERT ... #endif
> Please find updated webrev - http://cr.openjdk.java.net/~shshahma/8168914/webrev.01/

Looks good to me, Reviewed (I was the original author of the patch).
Thanks for taking care of the backport!

Erik

> Regards,
> Shafi
>
>> -----Original Message-----
>> From: David Holmes
>> Sent: Wednesday, April 26, 2017 12:09 PM
>> To: Shafi Ahmad <[hidden email]>; hotspot-
>> [hidden email]
>> Cc: Coleen Phillimore <[hidden email]>; Thomas Schatzl
>> <[hidden email]>; Stefan Karlsson
>> <[hidden email]>
>> Subject: Re: [8u] RFR for JDK-8168914: Crash in
>> ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking? to
>> jdk8u-dev
>>
>> Hi Shafi,
>>
>> On 26/04/2017 3:32 PM, Shafi Ahmad wrote:
>>> Hi,
>>>
>>> Please review the backport of bug: "JDK-8168914: Crash in
>>> ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking" to
>>> jdk8u-dev
>>>
>>> Please note that this is not a clean backport due to -
>>> A) Method  remove_handle(jobject h) is not present in jdk8u and looks it is
>> related to module so I haven't merged associated change to jdk8u.
>>
>> Without remove_handle_unsafe there is no use of "contains" so all of the
>> code in the block:
>>
>>   138 #ifdef ASSERT
>>   139 class VerifyContainsOopClosure : public OopClosure {
>>   ...
>>   167 #endif
>>
>> is dead code and can be deleted.
>>
>> Otherwise the backported code looks okay to me.
>>
>> Thanks,
>> David
>> -----
>>
>>> B) File moduleEntry.cpp is not present in jdk8u repo, so I have ignore its
>> changes.
>>>
>>>
>>> Webrev: http://cr.openjdk.java.net/~shshahma/8168914/webrev.00/
>>> jdk9 bug: https://bugs.openjdk.java.net/browse/JDK-8168914
>>> Original patch pushed to jdk9:
>>> http://hg.openjdk.java.net/jdk9/hs/hotspot/rev/606c35b6fac5
>>>
>>> Test:  Run jprt and jtreg is in progress.
>>>
>>> Regards,
>>> Shafi
>>>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RE: [8u] RFR for JDK-8168914: Crash in ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking? to jdk8u-dev

Shafi Ahmad
Thank you Eric for reviewing it.

Regards,
Shafi

> -----Original Message-----
> From: Erik Helin
> Sent: Wednesday, April 26, 2017 3:44 PM
> To: Shafi Ahmad <[hidden email]>; David Holmes
> <[hidden email]>; [hidden email]
> Subject: Re: [8u] RFR for JDK-8168914: Crash in
> ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking? to
> jdk8u-dev
>
> On 04/26/2017 09:43 AM, Shafi Ahmad wrote:
> > Hi David,
> >
> > Thank you for the review comment.
> >
> > I have deleted the redundant code from the cpp and hpp file under
> > #ifdef ASSERT ... #endif Please find updated webrev -
> > http://cr.openjdk.java.net/~shshahma/8168914/webrev.01/
>
> Looks good to me, Reviewed (I was the original author of the patch).
> Thanks for taking care of the backport!
>
> Erik
>
> > Regards,
> > Shafi
> >
> >> -----Original Message-----
> >> From: David Holmes
> >> Sent: Wednesday, April 26, 2017 12:09 PM
> >> To: Shafi Ahmad <[hidden email]>; hotspot-
> >> [hidden email]
> >> Cc: Coleen Phillimore <[hidden email]>; Thomas Schatzl
> >> <[hidden email]>; Stefan Karlsson
> >> <[hidden email]>
> >> Subject: Re: [8u] RFR for JDK-8168914: Crash in
> >> ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking? to
> >> jdk8u-dev
> >>
> >> Hi Shafi,
> >>
> >> On 26/04/2017 3:32 PM, Shafi Ahmad wrote:
> >>> Hi,
> >>>
> >>> Please review the backport of bug: "JDK-8168914: Crash in
> >>> ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking"
> >>> to jdk8u-dev
> >>>
> >>> Please note that this is not a clean backport due to -
> >>> A) Method  remove_handle(jobject h) is not present in jdk8u and
> >>> looks it is
> >> related to module so I haven't merged associated change to jdk8u.
> >>
> >> Without remove_handle_unsafe there is no use of "contains" so all of
> >> the code in the block:
> >>
> >>   138 #ifdef ASSERT
> >>   139 class VerifyContainsOopClosure : public OopClosure {
> >>   ...
> >>   167 #endif
> >>
> >> is dead code and can be deleted.
> >>
> >> Otherwise the backported code looks okay to me.
> >>
> >> Thanks,
> >> David
> >> -----
> >>
> >>> B) File moduleEntry.cpp is not present in jdk8u repo, so I have
> >>> ignore its
> >> changes.
> >>>
> >>>
> >>> Webrev: http://cr.openjdk.java.net/~shshahma/8168914/webrev.00/
> >>> jdk9 bug: https://bugs.openjdk.java.net/browse/JDK-8168914
> >>> Original patch pushed to jdk9:
> >>> http://hg.openjdk.java.net/jdk9/hs/hotspot/rev/606c35b6fac5
> >>>
> >>> Test:  Run jprt and jtreg is in progress.
> >>>
> >>> Regards,
> >>> Shafi
> >>>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [8u] RFR for JDK-8168914: Crash in ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking? to jdk8u-dev

David Holmes
In reply to this post by Shafi Ahmad
Looks good!

Thanks,
David

On 26/04/2017 5:43 PM, Shafi Ahmad wrote:

> Hi David,
>
> Thank you for the review comment.
>
> I have deleted the redundant code from the cpp and hpp file under #ifdef ASSERT ... #endif
> Please find updated webrev - http://cr.openjdk.java.net/~shshahma/8168914/webrev.01/
>
> Regards,
> Shafi
>
>> -----Original Message-----
>> From: David Holmes
>> Sent: Wednesday, April 26, 2017 12:09 PM
>> To: Shafi Ahmad <[hidden email]>; hotspot-
>> [hidden email]
>> Cc: Coleen Phillimore <[hidden email]>; Thomas Schatzl
>> <[hidden email]>; Stefan Karlsson
>> <[hidden email]>
>> Subject: Re: [8u] RFR for JDK-8168914: Crash in
>> ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking? to
>> jdk8u-dev
>>
>> Hi Shafi,
>>
>> On 26/04/2017 3:32 PM, Shafi Ahmad wrote:
>>> Hi,
>>>
>>> Please review the backport of bug: "JDK-8168914: Crash in
>>> ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking" to
>>> jdk8u-dev
>>>
>>> Please note that this is not a clean backport due to -
>>> A) Method  remove_handle(jobject h) is not present in jdk8u and looks it is
>> related to module so I haven't merged associated change to jdk8u.
>>
>> Without remove_handle_unsafe there is no use of "contains" so all of the
>> code in the block:
>>
>>   138 #ifdef ASSERT
>>   139 class VerifyContainsOopClosure : public OopClosure {
>>   ...
>>   167 #endif
>>
>> is dead code and can be deleted.
>>
>> Otherwise the backported code looks okay to me.
>>
>> Thanks,
>> David
>> -----
>>
>>> B) File moduleEntry.cpp is not present in jdk8u repo, so I have ignore its
>> changes.
>>>
>>>
>>> Webrev: http://cr.openjdk.java.net/~shshahma/8168914/webrev.00/
>>> jdk9 bug: https://bugs.openjdk.java.net/browse/JDK-8168914
>>> Original patch pushed to jdk9:
>>> http://hg.openjdk.java.net/jdk9/hs/hotspot/rev/606c35b6fac5
>>>
>>> Test:  Run jprt and jtreg is in progress.
>>>
>>> Regards,
>>> Shafi
>>>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RE: [8u] RFR for JDK-8168914: Crash in ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking? to jdk8u-dev

Shafi Ahmad
Hi David,

Thank you for the review.

Regards,
Shafi

> -----Original Message-----
> From: David Holmes
> Sent: Wednesday, April 26, 2017 5:35 PM
> To: Shafi Ahmad <[hidden email]>; hotspot-
> [hidden email]
> Cc: Coleen Phillimore <[hidden email]>; Thomas Schatzl
> <[hidden email]>; Stefan Karlsson
> <[hidden email]>
> Subject: Re: [8u] RFR for JDK-8168914: Crash in
> ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking? to
> jdk8u-dev
>
> Looks good!
>
> Thanks,
> David
>
> On 26/04/2017 5:43 PM, Shafi Ahmad wrote:
> > Hi David,
> >
> > Thank you for the review comment.
> >
> > I have deleted the redundant code from the cpp and hpp file under
> > #ifdef ASSERT ... #endif Please find updated webrev -
> > http://cr.openjdk.java.net/~shshahma/8168914/webrev.01/
> >
> > Regards,
> > Shafi
> >
> >> -----Original Message-----
> >> From: David Holmes
> >> Sent: Wednesday, April 26, 2017 12:09 PM
> >> To: Shafi Ahmad <[hidden email]>; hotspot-
> >> [hidden email]
> >> Cc: Coleen Phillimore <[hidden email]>; Thomas Schatzl
> >> <[hidden email]>; Stefan Karlsson
> >> <[hidden email]>
> >> Subject: Re: [8u] RFR for JDK-8168914: Crash in
> >> ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking? to
> >> jdk8u-dev
> >>
> >> Hi Shafi,
> >>
> >> On 26/04/2017 3:32 PM, Shafi Ahmad wrote:
> >>> Hi,
> >>>
> >>> Please review the backport of bug: "JDK-8168914: Crash in
> >>> ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking"
> >>> to jdk8u-dev
> >>>
> >>> Please note that this is not a clean backport due to -
> >>> A) Method  remove_handle(jobject h) is not present in jdk8u and
> >>> looks it is
> >> related to module so I haven't merged associated change to jdk8u.
> >>
> >> Without remove_handle_unsafe there is no use of "contains" so all of
> >> the code in the block:
> >>
> >>   138 #ifdef ASSERT
> >>   139 class VerifyContainsOopClosure : public OopClosure {
> >>   ...
> >>   167 #endif
> >>
> >> is dead code and can be deleted.
> >>
> >> Otherwise the backported code looks okay to me.
> >>
> >> Thanks,
> >> David
> >> -----
> >>
> >>> B) File moduleEntry.cpp is not present in jdk8u repo, so I have
> >>> ignore its
> >> changes.
> >>>
> >>>
> >>> Webrev: http://cr.openjdk.java.net/~shshahma/8168914/webrev.00/
> >>> jdk9 bug: https://bugs.openjdk.java.net/browse/JDK-8168914
> >>> Original patch pushed to jdk9:
> >>> http://hg.openjdk.java.net/jdk9/hs/hotspot/rev/606c35b6fac5
> >>>
> >>> Test:  Run jprt and jtreg is in progress.
> >>>
> >>> Regards,
> >>> Shafi
> >>>
Loading...