Hello,

Please review the below webrev incorporating David's comments.


http://cr.openjdk.java.net/~hb/8152589/webrev.01/

Regards

Harsha


On Wednesday 17 August 2016 11:45 AM, Harsha Wardhana B wrote:
Hi David,

I will incorporate changes suggested by you. Let's wait for few more review comments and then I will send consolidated webrev.

Regards
Harsha

On Wednesday 17 August 2016 09:02 AM, David Holmes wrote:
On 16/08/2016 11:33 PM, Harsha Wardhana B wrote:
Hi David,

Agreed that we could fix WaitingThread the way you have said, but in
recent past, there aren't any issues reported w.r.t WaitingThread.

Nor are there likely to be - that's what makes spurious wakeup bugs so difficult to detect!

This test has been fixed several times (3-4) for intermittent failures
and hence I would not like to meddle with code that is not causing any
problems even though there is scope for refactoring.

It isn't refactoring it is fixing and we have fixed several tests in a similar way.

The issue reported was with LockThreadB and hence I have provided
possible fix for the same.

That doesn't preclude fixing other issues with the test at the same time.

David

Thanks

Harsha


On Tuesday 16 August 2016 01:32 PM, David Holmes wrote:
Hi Harsha,

On 16/08/2016 4:08 PM, Harsha Wardhana B wrote:
Hello,

Please review and provide comments for fix for issue,

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

having webrev located at

http://cr.openjdk.java.net/~hb/8152589/webrev.00/

These changes look quite good (though I have to admit I struggle to
read the lambda and stream code :) ).

Note that like many of these kinds of tests there is an issue with
WaitingThread because it does not wait in a loop and so is susceptible
to spurious wakeups. The way to fix that is to add a "notified"
variable and then do:

while (!notified)
  wait();

and set notified before the notify().

Thanks,
David

Fix details:

1. From nightly failures we see that LockThreadB was blocked on wrong
object. We now do a repeated check with timeout if any given thread is blocked on expected object. It is possible that LockThreadB might still
be in Phaser call stack (Unsafe.park) when 'checkBlockedObject' is
invoked.

2. The logs from lock free logger was never printed. It is not being
printed.

3. Any time we see failure, thread stack is being logged. This helps us
ascertain if failure is in test case or in the component.

4. Even though we had lock free logger, several ex.printStackTrace() was
used which could be responsible for failure. It is removed.

5. There were several places where tests continue to ran even after
failure (testFailed flag). That is fixed.

6. Couple of other minor refactoring.

Thanks

Harsha




Reply via email to