[11] RFR(XS) 8202152: test/hotspot/jtreg/runtime/whitebox/WBStackSize.java fails

[David Holmes]

On 25/04/2018 1:34 PM, dean.long at oracle.com wrote:
> OK.  I guess another fix would be to have the test ignore 
> actualStackSize > configStackSize.

Yes - given the requested stacksizes are only minimums the test is 
expecting too much.

But as the test is run in othervm mode we don't have to worry about 
recycling a thread that may have had an explicit stack size set (via 
java.lang.Thread constructor) - at least not until some other part of 
the runtime innocently introduces one ...

David

> dl
> 
> On 4/24/18 2:30 PM, Vladimir Kozlov wrote:
>> I don't think pthreads from other tests/apps will have affect.
>> Stack sizes are set during startup regardless what will be run after 
>> that (one or several tests).
>>
>> We have only 3 (I think) stack size: VM (vm, gc, watcher - native 
>> threads), compiler, Java threads.
>>
>> The only concern that GC also now has dynamic threads creation. But I 
>> am not sure they release those threads.
>>
>> Note, in general it is not a problem to reuse bigger stacks.
>> The test is too strict assuming a new stack will always be created. 
>> That is a problem and it addressed by the fix.
>>
>> Thanks,
>> Vladimir
>>
>> On 4/24/18 1:20 PM, dean.long at oracle.com wrote:
>>> It seems like this could still break if the pthreads implementation 
>>> decides to hand out (non-recycled) threads with oversized stacks, or 
>>> if there are recycled threads that don't use the values from the 
>>> command-line.  What happens if we run lots of tests in samevm mode, 
>>> and those tests uses different values for the stack size?  Then 
>>> wouldn't pthreads have a cache of various stack sizes to recycle new 
>>> threads from?
>>>
>>> dl
>>>
>>> On 4/23/18 11:12 PM, David Holmes wrote:
>>>> On 24/04/2018 3:54 PM, David Holmes wrote:
>>>>> To follow up ... Vladimir and I have had an interesting back and 
>>>>> forth in the bug report. It seems that at least in some cases 
>>>>> terminated threads are getting recycled - not just their pthread_t 
>>>>> identities but the actual allocated stack region as well. We seem 
>>>>> to end up with a new regular Java thread acquiring the id and stack 
>>>>> of a just terminated compiler thread - hence the regular Java 
>>>>> thread gets the wrong size stack!
>>>>>
>>>>> This is quite bizarre and seems quite broken.
>>>>
>>>> No it's not broken. The stacksize attribute is just a minimum size, 
>>>> so the OS can reuse a terminated thread with a larger stack - and 
>>>> that's what we are seeing.
>>>>
>>>> So the fix is fine, I would just add before @run:
>>>>
>>>> @comment Must use the same stacksize for Java threads and compiler 
>>>> threads in case of thread recycling
>>>>
>>>> Or something to that affect.
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> David
>>>>>
>>>>> On 24/04/2018 7:24 AM, David Holmes wrote:
>>>>>> Hi Vladimir,
>>>>>>
>>>>>> On 24/04/2018 5:20 AM, Vladimir Kozlov wrote:
>>>>>>> http://cr.openjdk.java.net/~kvn/8202152/webrev.00/
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8202152
>>>>>>>
>>>>>>> After 8198756 changes compiler threads could be released and 
>>>>>>> other Java threads can re-use the same HW threads for which stack 
>>>>>>> size was determined during start. The test WBStackSize.java may 
>>>>>>> fail in such cases because Java thread stack size is set and 
>>>>>>> checked in test.
>>>>>>
>>>>>> That does not make sense to me. We don't "re-use HW threads" - I'm 
>>>>>> not even sure what that means. Compiler threads have a different 
>>>>>> stacksize to other Java threads - that seems simple enough.
>>>>>>
>>>>>>> For 8198756 changes we added thread name check to workaround the 
>>>>>>> problem [1]. But it seems is not enough.
>>>>>>
>>>>>> Why would this test be run in a compiler thread ????
>>>>>>
>>>>>>> To avoid this issue I added -XX:CompilerThreadStackSize=512 flag 
>>>>>>> to match Java thread stack size (note, size is in Kb for this flag).
>>>>>>
>>>>>> That seems like a workaround of the real problem.
>>>>>>
>>>>>> David
>>>>>> -----
>>>>>>
>>>>>>> I also reverted changes in test code and instead added thread 
>>>>>>> name print to know in case we still have problem in a future.
>>>>>>>
>>>>>>> Tier1 testing (which run the test) passed.
>>>>>>>
>>>
>