RFR (M): 8145435: [JVMCI] some tests on Windows fail with: assert(!thread->is_Java_thread()) failed: must not be java thread

[Christian Thalinger]

> On Dec 17, 2015, at 9:17 AM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:
> 
> 
> 
> On 12/17/15 2:08 PM, Christian Thalinger wrote:
>>> On Dec 17, 2015, at 9:03 AM, Tom Rodriguez <tom.rodriguez at oracle.com> wrote:
>>> 
>>> 
>>>> On Dec 17, 2015, at 10:53 AM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
>>>> 
>>>> 
>>>>> On Dec 17, 2015, at 8:28 AM, Tom Rodriguez <tom.rodriguez at oracle.com> wrote:
>>>>> 
>>>>> I think the ttyLocker should be in print_stack_trace.
>>>> Well, the problem is that ttyLocker only locks defaultStream::instance (whatever that is) but print_stack_trace can take any outputStream.
>>> Well maybe it shouldn’t.  No one uses that flexibility but having a ttyLocker would be practically useful.
>> True.  I’m tempted to make that change but I see more bike-shedding coming up. Maybe if Coleen is okay with that… ;-)
> 
> No, just leave it where it was....   With logging the ttyLocker doesn't work anyway.

Well, turns out that we can’t hold the tty lock when calling out to Java:

https://bugs.openjdk.java.net/browse/JDK-8146246

Either we give up the lock completely or we call out to Java code to do the printing for us (which was there in the first place).  Since we are already calling out to Java (to Throwable.getCause) calling Java code at that point must work.

> 
> Coleen
>> 
>>>  Anyway, it’s ok as is too.
>>> 
>>> tom
>>> 
>>>>> A short comment on print_stack_trace explaining that it prints the exception message along with the exception chain wouldn’t hurt.  Otherwise looks good.
>>>> Sure.
>>>> 
>>>>> tom
>>>>> 
>>>>>> On Dec 17, 2015, at 9:46 AM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
>>>>>> 
>>>>>>> On Dec 17, 2015, at 3:56 AM, Doug Simon <doug.simon at oracle.com <mailto:doug.simon at oracle.com>> wrote:
>>>>>>> 
>>>>>>>> On 17 Dec 2015, at 08:20, Tom Rodriguez <tom.rodriguez at oracle.com> wrote:
>>>>>>>> 
>>>>>>>>>>> I feel like there was a reason we weren’t using java_lang_Throwable::print_stack_trace but it looks like it handles the causes properly which is the only reason I could think of. Maybe Doug knows?
>>>>>>>>>> 
>>>>>>>>>> There are two issues with java_lang_Throwable::print_stack_trace:
>>>>>>>>>> 
>>>>>>>>>> 1. It doesn’t print the exception message, just the stack.
>>>>>>>>> I’m printing the message separately:
>>>>>>>>> 
>>>>>>>>> java_lang_Throwable::print(exception, tty);
>>>>>>>>> tty->cr();
>>>>>>>>> java_lang_Throwable::print_stack_trace(exception(), tty);
>>>>>>>> There are already 3 other copies of this idiom.  Throwable should probably have a single method for this.  Or maybe print_stack_trace itself should do this?
>>>>>>> +1
>>>>>>> 
>>>>>>>> That would better parallel Throwable.printStackTrace().
>>>>>>> Makes sense to me!
>>>>>> It does.  Here it goes:
>>>>>> 
>>>>>> http://cr.openjdk.java.net/~twisti/8145435/webrev.02/ <http://cr.openjdk.java.net/~twisti/8145435/webrev.02/>
>>>>>> 
>>>>>> I’ve made java_lang_Throwable::print_stack_trace to take a Handle instead of an oop and removed the unneeded copy of java_lang_Throwable::print.
>>>>>> 
>>>>>>> -Doug