RFR(S): JDK-8137035 tests got EXCEPTION_STACK_OVERFLOW on Windows 64 bit
Frederic Parain
frederic.parain at oracle.com
Wed Sep 7 16:48:35 UTC 2016
David,
Thank you for your review.
The consequence of "_thread_in_Java -> in_java" is just
some stack walking when stack overflows occur in VM or
native code, cases where the JVM is already in a very
risky situation anyway.
Fred
On 09/05/2016 08:13 PM, David Holmes wrote:
> On 3/09/2016 1:20 AM, Frederic Parain wrote:
>> Hi David,
>>
>> Here's an updated version of the fix:
>> http://cr.openjdk.java.net/~fparain/8137035/webrev.02/
>
> Thanks - looks good!
>
> I do have wonder what the consequence of the "_thread_in_Java ->
> in_java" may have been ??
>
> David
> -----
>
>> Regards,
>>
>> Fred
>>
>> On 09/01/2016 03:29 AM, David Holmes wrote:
>>> On 31/08/2016 9:04 AM, Frederic Parain wrote:
>>>> Hi David,
>>>>
>>>> On 08/29/2016 11:10 PM, David Holmes wrote:
>>>>> Hi Fred,
>>>>
>>>>>>> While examining the thread state logic in the exception handler I
>>>>>>> noticed some pre-existing bugs:
>>>>>>>
>>>>>>> 2506 if (exception_code == EXCEPTION_ACCESS_VIOLATION) {
>>>>>>> 2507 JavaThread* thread = (JavaThread*) t;
>>>>>>>
>>>>>>> there is no check that t is in fact a JavaThread, or even that t is
>>>>>>> non-NULL. Such checks occur slightly later:
>>>>>>
>>>>>> I've investigated this issue, and it is currently harmless.
>>>>>> The casted pointer is only used to call a method requiring
>>>>>> a JavaThread* pointer and the only usage of its argument it's
>>>>>> a NULL check. Unfortunately, fixing this issue would require
>>>>>> to modify the prototype of os::is_memory_serialize_page()
>>>>>> and propagate the change across all platforms using it.
>>>>>> It's a wider scope fix than JDK-8137035.
>>>>>
>>>>> I was only expecting you to move (if the scoping allows it, else copy)
>>>>> the later:
>>>>
>>>> The scoping doesn't allow his move, and adding the test would
>>>> potentially change the behavior (I'm expecting that only JavaThreads
>>>> could be blocked on the serialization page, but I might be wrong).
>>>
>>> If we were to find that a non-JavaThread were executing that code that
>>> would be a significant bug I think. I really think this needs to be
>>> fixed to ensure it is a JavaThread - even if you only add an assert.
>>>
>>> Thanks,
>>> David
>>>
>>>> Fred
>>>>
>>>>> if (t != NULL && t->is_Java_thread()) {
>>>>>
>>>>> check. ;)
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> I've added a comment the unsafe cast in os_windows.cpp file,
>>>>>> highlighting the fact it was unsafe, and explaining why it
>>>>>> is currently harmless.
>>>>>>
>>>>>>>
>>>>>>> 2523 if (t != NULL && t->is_Java_thread()) {
>>>>>>> 2524 JavaThread* thread = (JavaThread*) t;
>>>>>>>
>>>>
More information about the hotspot-runtime-dev
mailing list