This approach looks good to me. Please go ahead and update 7123972 as well, and
I'll post these webrevs tomorrow (my time).
Thanks.
s'marks
On 7/2/12 11:41 PM, Eric Wang wrote:
David,
I have added the comments before the loop, please help to review. If it is OK,
I'll update the 7123972 as well.
Thanks,
Eric
On 2012/7/3 12:22, David Holmes wrote:
Hi Eric,
On 3/07/2012 1:28 PM, Eric Wang wrote:
Hi Stuart and David,
Thanks for the suggestion about the timeout. so there are three ways to
handle the timeout.
1. Add comment to explain that test failed due to default timeout
2. Specify the the timeout option
3. Timeout programly
Is it ok for you that i'd like to choose the second way "explict
timeout" as the test looks simply and lucid?
If by #2 you mean add something like:
@run main/timeout=10
then no. Earlier this year there were changes to a bunch of tests that set
timeouts shorter than the jtreg default, to delete the timeouts. Given that,
my misgivings about relying on the harness are not relevant so we need only
do #1 and add a comment to make it clear that a failing test will be
indicated via the harness timeout.
David
-----
Regards,
Eric
On 2012/7/3 10:38, David Holmes wrote:
On 3/07/2012 7:08 AM, Stuart Marks wrote:
On 7/1/12 5:39 PM, David Holmes wrote:
Now, how can the test fail? If ref is never cleared, the while loop
will
never terminate, and we rely on jtreg to timeout and kill this
test. It
would be good to have a brief comment above the while loop that
explains
this.
Agreed. Something like
// Might require multiple calls to System.gc() for weak-references
processing
to be complete.
// If the weak-reference is not cleared as expected we will hang here
// until timed out by the test harness
Though now I write that I'm unhappy that this will only behave nicely
if run
from jtreg. We do use explicit timeouts in other tests.
So, are you unhappy enough that we should ask Eric to add an explicit
timeout?
No. But I'm not the authoritive voice in this area :)
I don't think it would be that difficult. Perhaps a technique like the
following would suffice. Get System.currentTimeMillis() before the loop,
and call it again each time through the loop, and fail if the difference
is greater than the timeout (e.g., 60 sec). Doesn't involve other
threads or even Timer/TimerTask.
Or simply limit the number of loops so that N*sleepTime = timeout.
This then requires adding back the check for null outside the loop.
On the other hand if one is running this test directly instead of
through jtreg, one can just ^C it if it's taking an unexpectedly long
time.
True.
David
-----
s'marks
