'Re: RFR: JDK-8218128: vmTestbase/nsk/jvmti/ResourceExhausted/resexhausted003 and 004 use wrong path'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: JDK-8218128: vmTestbase/nsk/jvmti/ResourceExhausted/resexhausted003 and 004 use wrong path
From:       "gary.adams () oracle ! com" <gary ! adams () oracle ! com>
Date:       2019-03-26 9:41:14
Message-ID: 5bdae6ca-a4eb-2ab2-b197-31511fd64245 () oracle ! com
[Download RAW message or body]

The stab at catching the OOME was added to try an diagnose some of the 
remaininf failures.
The tests will remain on the ProblemList.

Probably best to remove the extra debugging and start fresh when the 
other issues are
investigated.

On 3/26/19 1:56 AM, David Holmes wrote:
> On 26/03/2019 3:34 pm, Chris Plummer wrote:
>> On 3/25/19 10:11 PM, David Holmes wrote:
>>> On 26/03/2019 3:05 pm, Chris Plummer wrote:
>>>> printStackTrace shouldn't ever result in an OOME since the memory 
>>>> held that caused the previous OOME should be released before 
>>>> another OOME is generated (no longer any references to it).
>>>
>>> It will depend on the code executed leading up to and after the OOME 
>>> that will determine if memory has been released by the time the OOME 
>>> reaches the top-level.
>> Well, my assumption is that there is no class static holding on to 
>> the memory. If there isn't and printStackTrace needs to allocate an 
>> object, then a gc will be done if necessary. If there is static, I'd 
>> call that a test bug. A test shouldn't fill the heap, generate and 
>> catch an OOME, and then expect the test to be able to exit gracefully 
>> unless it first releases the memory.
>
> I think that was Leonid's point too. It's not obvious from 
> resexhausted004.java that that is the case.
>
> That aside the change to resexhausted004.java seems unnecessary. These 
> tests report success or failure by calling System.exit() with an 
> appropriate return code. By catching the exceptions the VM will appear 
> to exit with a zero code, which should still be seen as a failure in 
> this case. If the exceptions are left to propagate out you'll get a 
> non-zero return code which should still be a failure, and the 
> stacktrace printing will be handled in the normal way if main returns 
> via throwing an exception.
>
> David
>
>> Chris
>>>
>>> David
>>>
>>>> Chris
>>>>
>>>> On 3/25/19 9:56 PM, Leonid Mesnik wrote:
>>>>> Gary
>>>>>
>>>>> Just want to add that following code
>>>>> 68 } catch (OutOfMemoryError me) {
>>>>> 69 me.printStackTrace();
>>>>> 70 }
>>>>> might throw another OOME in printStackTrace().
>>>>>
>>>>> If you want to do something after OOME it is needed to free some 
>>>>> before trying to allocate.
>>>>>
>>>>> Leonid
>>>>>
>>>>>> On Mar 25, 2019, at 12:46 PM, Chris Plummer 
>>>>>> <chris.plummer@oracle.com <mailto:chris.plummer@oracle.com>> wrote:
>>>>>>
>>>>>> Hi Gary,
>>>>>>
>>>>>> In resexhausted004.java, the indentation needs to be fixed. Also, 
>>>>>> what happens if one of the caught exceptions is thrown? Does the 
>>>>>> test fail or pass? I'm not sure what the default behavior is if 
>>>>>> you just return from main().
>>>>>>
>>>>>> Otherwise looks good.
>>>>>>
>>>>>> thanks,
>>>>>>
>>>>>> Chris
>>>>>>
>>>>>> On 3/25/19 11:18 AM, Gary Adams wrote:
>>>>>>> During testing I found that there is still good reasons to leave 
>>>>>>> 003 and 004
>>>>>>> tests on the ProblemList. I believe there is still work to be 
>>>>>>> done to ensure
>>>>>>> JDK-7013634 and JDK-6606767 are covered.
>>>>>>>
>>>>>>> This pass attempts to solve the primary issue in this bug which 
>>>>>>> is improper paths on the
>>>>>>> test command lines. Cleanup is limited to the 003 and 004 tests. 
>>>>>>> The tests will
>>>>>>> remain on the ProblemList.
>>>>>>>
>>>>>>>    Issue: https://bugs.openjdk.java.net/browse/JDK-8218128
>>>>>>>    Webrev: 
>>>>>>> http://cr.openjdk.java.net/~gadams/8218128/webrev.01/index.html
>>>>>>>
>>>>>>> On 3/8/19, 3:39 PM, Chris Plummer wrote:
>>>>>>>> Ok, I didn't realize the @ignore was removed also. I guess this 
>>>>>>>> is what "exclude" was converted into. So the question now is 
>>>>>>>> should the @ignore (and the exclude comment) remain. The 
>>>>>>>> @ignore bug (JDK-7013634) was closed as CNR. I'm actually the 
>>>>>>>> one that closed it a while back, and that may have been a 
>>>>>>>> mistake since I didn't realize that some of the tests had been 
>>>>>>>> completely excluded from testing, even quarantine testing. 
>>>>>>>> After reading through some of the comments in JDK-7013634, I'm 
>>>>>>>> not trusting these tests to run reliably. That's often the case 
>>>>>>>> with tests that try to exhaust resources.
>>>>>>>>
>>>>>>>> Chris
>>>>>>>>
>>>>>>>> On 3/8/19 9:41 AM, Gary Adams wrote:
>>>>>>>>> On 3/8/19, 12:10 PM, Chris Plummer wrote:
>>>>>>>>>> You can remove the quarantine and exclude keywords. I think 
>>>>>>>>>> that's appropriate if the test is off the problemlist and 
>>>>>>>>>> working. It was just nonconcurrent removal that I was against.
>>>>>>>>> Done
>>>>>>>>>>
>>>>>>>>>> As for the resexhausted001 failure you are still seeing, how 
>>>>>>>>>> could jtreg exclude it if it was not on the problemlist. I 
>>>>>>>>>> didn't think there was any other mechanism. I don't believe 
>>>>>>>>>> jtreg looks at the tonga keywords.
>>>>>>>>>
>>>>>>>>> I believe the @ignore is a jtreg keyword.
>>>>>>>>> With the -ignore command line flag jtreg can be directed to
>>>>>>>>> quietly ignore a test, or force an error, or attempt to run 
>>>>>>>>> the test
>>>>>>>>> even though the @ignore directive is there.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> thanks,
>>>>>>>>>>
>>>>>>>>>> Chris
>>>>>>>>>>
>>>>>>>>>> On 3/8/19 5:07 AM, Gary Adams wrote:
>>>>>>>>>>> I'll revert the comments that were left in during the tonga 
>>>>>>>>>>> conversion.
>>>>>>>>>>>
>>>>>>>>>>> I did come across an interesting test failure in 
>>>>>>>>>>> resexhausted001
>>>>>>>>>>> which had an
>>>>>>>>>>>    @ignore 7013634
>>>>>>>>>>>
>>>>>>>>>>>        JDK-7013634: jvmti resexhausted001 can timeout or fail 
>>>>>>>>>>> due to excessive thread creation
>>>>>>>>>>>
>>>>>>>>>>> The test was closed because it was not reproducible.
>>>>>>>>>>> Even though the test was not on the ProblemList, I believe
>>>>>>>>>>> jtreg was excluding the test from running.
>>>>>>>>>>>
>>>>>>>>>>> The original problem reported an "out of swap" condition.
>>>>>>>>>>>
>>>>>>>>>>> The current failure reports:
>>>>>>>>>>> ----------System.out:(3/217)----------
>>>>>>>>>>> Creating threads...
>>>>>>>>>>> Timeout refired 480 times
>>>>>>>>>>> [730.871s][warning][os,thread] Failed to start thread - 
>>>>>>>>>>> _beginthreadex failed (EACCES) for attributes: stacksize: 
>>>>>>>>>>> default, flags: CREATE_SUSPENDED STACK_SIZE_PARAM_IS.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 3/7/19, 4:02 PM, Chris Plummer wrote:
>>>>>>>>>>>> Hi Gary,
>>>>>>>>>>>>
>>>>>>>>>>>> Why did you remove the "nonconcurrent" keyword. I know 
>>>>>>>>>>>> these are just comments for reference that were added when 
>>>>>>>>>>>> the test was ported from tonga, but as a comment it is 
>>>>>>>>>>>> still applicable. The test should not be run concurrent 
>>>>>>>>>>>> with others (which you have also fixed with the addition of 
>>>>>>>>>>>> the "exclusiveAccess.dirs=.").
>>>>>>>>>>>>
>>>>>>>>>>>> Otherwise changes look good.
>>>>>>>>>>>>
>>>>>>>>>>>> thanks,
>>>>>>>>>>>>
>>>>>>>>>>>> Chris
>>>>>>>>>>>>
>>>>>>>>>>>> On 3/7/19 10:57 AM, Gary Adams wrote:
>>>>>>>>>>>>> This proposed fix will restore the ResourceExhausted tests.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Test 3 and 4 were on the ProblemList because of the potential
>>>>>>>>>>>>> path issues in finding the correct classes. This change 
>>>>>>>>>>>>> searches the
>>>>>>>>>>>>> test.class.path for the appropriate vmTestbase classes 
>>>>>>>>>>>>> rather than using
>>>>>>>>>>>>> incorrect settings on the command line.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Some clean up has been done to remove quarantine keyword
>>>>>>>>>>>>> and @ignore directives. Should additional clean up be done 
>>>>>>>>>>>>> to remove
>>>>>>>>>>>>> bug numbers, etc.?
>>>>>>>>>>>>>
>>>>>>>>>>>>> TEST.PROPERTIES were added so test 3 so it is consistent 
>>>>>>>>>>>>> with the other tests
>>>>>>>>>>>>> in the group.
>>>>>>>>>>>>>
>>>>>>>>>>>>>    Issue: https://bugs.openjdk.java.net/browse/JDK-8218128
>>>>>>>>>>>>>    Webrev: 
>>>>>>>>>>>>> http://cr.openjdk.java.net/~gadams/8218128/webrev.00/index.html 
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Local testing has been successful on a linux-x64-debug build.
>>>>>>>>>>>>> Testing on mach5 for other platforms next.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>
>>

[prev in list] [next in list] [prev in thread] [next in thread]
Configure | About | News | Add a list | Sponsored by KoreLogic