RFR(S): JDK-8137035 tests got EXCEPTION_STACK_OVERFLOW on Windows 64 bit

[David Holmes]

On 8/09/2016 2:48 AM, Frederic Parain wrote:
> 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.

Just wondering if it might explain any odd crashes we had seen. :)

Cheers,
David

> 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;
>>>>>>>>
>>>>>